thoughts? changing the type of SidecarsHooks for corev1.Pod

48 views
Skip to first unread message

Alexander Gallego

unread,
Jan 3, 2019, 2:50:35 PM1/3/19
to kubevirt-dev
Hi Guys, 

Doing some more advanced integration w/ the sidecar hooks and was wondering if you'd welcome a change of the type of the sidecars hook.

Currently the code is this: 


type HookSidecarList []HookSidecar

type HookSidecar struct {
Image           string           `json:"image"`
ImagePullPolicy k8sv1.PullPolicy `json:"imagePullPolicy"`
}

func UnmarshalHookSidecarList(vmiObject *v1.VirtualMachineInstance) (HookSidecarList, error) {
hookSidecarList := make(HookSidecarList, 0)

if rawRequestedHookSidecarList, requestedHookSidecarListDefined := vmiObject.GetAnnotations()[HookSidecarListAnnotationName]; requestedHookSidecarListDefined {
if err := json.Unmarshal([]byte(rawRequestedHookSidecarList), &hookSidecarList); err != nil {
return nil, err
}
}

return hookSidecarList, nil
}




The issue is that I need to pass the sidecar hook a config-map object *per* sidecar hook. 

Some design has to go in here, but wondering how do you envision passing code. 

The only alternative is for me to add a json annotation just like the demo does here:


```
... snip

kind: VirtualMachineInstance
metadata:
  annotations:
    hooks.kubevirt.io/hookSidecars: '[{"image": "registry:5000/kubevirt/example-hook-sidecar:devel"}]'
  creationTimestamp: null
  labels:
    special: vmi-with-sidecar-hook
  name: vmi-with-sidecar-hook

....
```

So that way i can serialize my config from an annotation, which is OK but less ideal. 

Thoughts? 

Alexander Gallego

unread,
Jan 3, 2019, 2:53:41 PM1/3/19
to kubevirt-dev


On Thursday, January 3, 2019 at 2:50:35 PM UTC-5, Alexander Gallego wrote:
Hi Guys, 

Doing some more advanced integration w/ the sidecar hooks and was wondering if you'd welcome a change of the type of the sidecars hook.

Currently the code is this: 


type HookSidecarList []HookSidecar

type HookSidecar struct {
Image           string           `json:"image"`
ImagePullPolicy k8sv1.PullPolicy `json:"imagePullPolicy"`
}

func UnmarshalHookSidecarList(vmiObject *v1.VirtualMachineInstance) (HookSidecarList, error) {
hookSidecarList := make(HookSidecarList, 0)

if rawRequestedHookSidecarList, requestedHookSidecarListDefined := vmiObject.GetAnnotations()[HookSidecarListAnnotationName]; requestedHookSidecarListDefined {
if err := json.Unmarshal([]byte(rawRequestedHookSidecarList), &hookSidecarList); err != nil {
return nil, err
}
}

return hookSidecarList, nil
}




The issue is that I need to pass the sidecar hook a config-map object *per* sidecar hook. 

Some design has to go in here, but wondering how do you envision passing code. 

s/envision passing code/envision passing configurations to sidecar hooks/

Petr Kotas

unread,
Jan 4, 2019, 7:24:18 AM1/4/19
to Alexander Gallego, kubevirt-dev
Hi Alexander,

We are open to enhancements. Would you please create pull request?
It is easier to review the idea there.

Thanks a lot.
Petr

--
You received this message because you are subscribed to the Google Groups "kubevirt-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubevirt-dev...@googlegroups.com.
To post to this group, send email to kubevi...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubevirt-dev/8314c864-0e1e-47c1-8f24-91921c579e1c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Fabian Deutsch

unread,
Jan 4, 2019, 8:22:08 AM1/4/19
to Petr Kotas, Alexander Gallego, kubevirt-dev

Yo Alex,

can you give some more context of what you are trying to pass?

I'm asking to understand where this is going if it's more about additional data for a container or if it's about more flexibility in defining the containers.

- fabian

Alexander Gallego

unread,
Jan 4, 2019, 9:46:11 PM1/4/19
to Fabian Deutsch, Petr Kotas, kubevirt-dev, Michael Merideth
On Fri, Jan 4, 2019 at 8:22 AM Fabian Deutsch <fdeu...@redhat.com> wrote:

Yo Alex,

can you give some more context of what you are trying to pass?

I'm asking to understand where this is going if it's more about additional data for a container or if it's about more flexibility in defining the containers.


Sure thing. 

So on our sidecarhooks, we need to configure the hook to change the domain xml settings. 

currently, the sample project uses labels to do 2 things. 

1) choose the image the sidecar will run.
2) pass configuration to the sidecar via a label which is a bit inflexible.

So what we do today is we pass a json object encoded as a string in the labels to pass the information we need for the sidecarhook to use as input.

The proposal is roughly: 

if we move the type from: 

type HookSidecar struct {
Image           string           `json:"image"`
ImagePullPolicy k8sv1.PullPolicy `json:"imagePullPolicy"`
}


to 

type HookSidecar corev1.Pod

then we can configure the sidecarhook with a ConfigMap object or so.


Does this make sense?

.alex

aluk...@redhat.com

unread,
Jan 6, 2019, 3:18:20 AM1/6/19
to kubevirt-dev
I prefer just to extend current struct with the config map field because this struct does not really need to mirror all corev1.Pod fields.
Will it be enough for you?

Alexander Gallego

unread,
Jan 6, 2019, 3:28:08 PM1/6/19
to kubevirt-dev


On Sunday, January 6, 2019 at 3:18:20 AM UTC-5, aluk...@redhat.com wrote:
I prefer just to extend current struct with the config map field because this struct does not really need to mirror all corev1.Pod fields.
Will it be enough for you?


Not sure which thread you were responding to since this is a top post. 

But you are right, I was thinking v1.Container not Pod. 



We'll also need to pass in environment variables. 
Reply all
Reply to author
Forward
0 new messages