Regexp matching

[azr]

Hi there,

I'm about to start regexp matching, I have time for this 😄.

But I wanted to discuss some yaml first, so I'm going to make a full proposition here using examples:

Basic regex:

kind: Deployment
metadata:
  name: search
  labels:
    keel.sh/match: ":2(\.\d+\.\d+)?$" # :2 or :2.2.2 or :2.3.4
spec:
  template:
    spec:
      containers:
        - name: search
          image: acme/search:2.1.0
          imagePullPolicy: Always

---

We could use text/template and define/generate some regexps:

• {{ .Semver }} -> d+\.d+\.d+ ( or something better )
• {{ .Patch }} -> 2\.1\.\d+ ( Patch string is generated from container's image )
• {{ .Minor }} -> 2\.\d+\.\d+

It would be kinda easy to unit test.

---

Sidecars:
When there is more that one container we can be selective in the setting:

kind: Deployment
metadata:
  name: search
  labels:
    keel.sh/match: "/search:{{ .Major }}|/notsearch:{{ .Major }}"
spec:
  template:
    spec:
      containers:
        - name: search
          image: acme/search:2.1.0
          imagePullPolicy: Always
        - name: notsearch
          image: acme/notsearch:3.2.0
          imagePullPolicy: Always

Edit: tell me what you think :D

[rusenask]

Hi, regexp looks good. Labels are quite sensitive to what you can add there so you would have to use annotations.

Maybe we could support setting match regexp on pod spec as well? this way each pod with different image could have its own regexp that overrides the deployment/daemonset/stateful regexp. GetSpecAnnotations: https://github.com/keel-hq/keel/blob/master/internal/k8s/resource.go#L168-L178

[azr]

Don't forget that a pod can contain multiple containers.
I was trying to look at what can be done in container-specs so that they are updated selectively, but every option I though of was a bit soso to me;
because the container spec doesn't have any metada field to set ( yet ? )

container.go

type Container struct {
    
	Name string `json:"name" protobuf:"bytes,1,opt,name=name"`
    
	Image string `json:"image,omitempty" protobuf:"bytes,2,opt,name=image"`
    
	Command []string `json:"command,omitempty" protobuf:"bytes,3,rep,name=command"`
    
	Args []string `json:"args,omitempty" protobuf:"bytes,4,rep,name=args"`
    
	WorkingDir string `json:"workingDir,omitempty" protobuf:"bytes,5,opt,name=workingDir"`
    
	Ports []ContainerPort `json:"ports,omitempty" patchStrategy:"merge" patchMergeKey:"containerPort" protobuf:"bytes,6,rep,name=ports"`
    
	EnvFrom []EnvFromSource `json:"envFrom,omitempty" protobuf:"bytes,19,rep,name=envFrom"`
    
	Env []EnvVar `json:"env,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,7,rep,name=env"`
    
	Resources ResourceRequirements `json:"resources,omitempty" protobuf:"bytes,8,opt,name=resources"`
    
	VolumeMounts []VolumeMount `json:"volumeMounts,omitempty" patchStrategy:"merge" patchMergeKey:"mountPath" protobuf:"bytes,9,rep,name=volumeMounts"`
    
	LivenessProbe *Probe `json:"livenessProbe,omitempty" protobuf:"bytes,10,opt,name=livenessProbe"`
    
	ReadinessProbe *Probe `json:"readinessProbe,omitempty" protobuf:"bytes,11,opt,name=readinessProbe"`
    
	Lifecycle *Lifecycle `json:"lifecycle,omitempty" protobuf:"bytes,12,opt,name=lifecycle"`
    
	TerminationMessagePath string `json:"terminationMessagePath,omitempty" protobuf:"bytes,13,opt,name=terminationMessagePath"`
    
	TerminationMessagePolicy TerminationMessagePolicy `json:"terminationMessagePolicy,omitempty" protobuf:"bytes,20,opt,name=terminationMessagePolicy,casttype=TerminationMessagePolicy"`
    
	ImagePullPolicy PullPolicy `json:"imagePullPolicy,omitempty" protobuf:"bytes,14,opt,name=imagePullPolicy,casttype=PullPolicy"`
    
	SecurityContext *SecurityContext `json:"securityContext,omitempty" protobuf:"bytes,15,opt,name=securityContext"`




	Stdin bool `json:"stdin,omitempty" protobuf:"varint,16,opt,name=stdin"`
    
	StdinOnce bool `json:"stdinOnce,omitempty" protobuf:"varint,17,opt,name=stdinOnce"`
    
	TTY bool `json:"tty,omitempty" protobuf:"varint,18,opt,name=tty"`
}

It would be super awesome if somehow we could add parameters to the image field :

kind: Deployment
metadata:
  name: search
  labels:
    keel.sh/policy: "contextualized"
spec:
  template:
    spec:
      containers:
        - name: search
          image: acme/search:2.1.0?update={{ .Major }}
          imagePullPolicy: Always
        - name: notsearch
          image: acme/notsearch:3.2.0?update={{ .Minor }}
          imagePullPolicy: Always

But this causes an invalid image error.

One other dirty option would be to use env vars:

kind: Deployment
metadata:
  name: search
  labels:
    keel.sh/policy: "contextualized"
spec:
  template:
    spec:
      containers:
        - name: search
          image: acme/search:2.1.0
          imagePullPolicy: Always
          env:
          - name: KEEL_SH_UPGRADE_REGEX
             value: "{{ .Major }}"
        - name: notsearch
          image: acme/notsearch:3.2.0
          imagePullPolicy: Always
          env:
          - name: KEEL_SH_UPGRADE_REGEX
             value: "{{ .Minor }}"

But meh.

[azr]

It would be great to be able to extend image pull policy though:

kind: Deployment
metadata:
  name: search
spec:
  template:
    spec:
      containers:
        - name: search
          image: acme/search:2.1.0
          imagePullPolicy: "{{ .Major }}"
        - name: notsearch
          image: acme/notsearch:3.2.0
          imagePullPolicy: "{{ .Minor }}"

[rusenask]

how often would end up with a sidecar containers that can be updated by the events? 🤔

Maybe initial stab could be just for deployment wide regexp matcher? Or maybe matcher could also include image name too

[azr]

In my case: we have istio setup and all of our pods have a sidecar, and I would like them to be never updated.

Oh, I like it; So, using container name, something like:

kind: Deployment
metadata:
  name: search
  labels:
    keel.sh/policy: "regex"
    keel.sh/search: "{{ .Major }}"
    keel.sh/notsearch: "{{ .Minor }}
spec:
  template:
    spec:
      containers:
        - name: search
          image: acme/search:2.1.0
          imagePullPolicy: Always
        - name: notsearch
          image: acme/notsearch:3.2.0
          imagePullPolicy: Always

[rusenask]

Sorry, thought I replied to this (at least github was showing it for me :/ ).

How about we add an optional whitelist which images should be updated? This would be useful for all other policies too. Not sure whether we should use image names there or container names though (probably image names for clarity).

kind: Deployment
metadata:
  name: search
  labels:
    keel.sh/policy: "regex"
    keel.sh/match: "{{ .Major }}"    
  annotations:
    keel.sh/images: "acme/dogs,acme/cats" # <- by default empty whitelist would allow everything
spec:
  template:
    spec:
      containers:
        - name: dogservice
          image: acme/dogs:2.1.0
          imagePullPolicy: Always
        - name: catservice
          image: acme/cats:3.2.0
          imagePullPolicy: Always
        - name: horservice 
          image: acme/horses:3.2.0 # <- horses would be updated
          imagePullPolicy: Always

[RXminuS]

@rusenask how about adding different match labels for each container?

labels:
    keel.sh/match: "{{ .Minor }}" #default
    keel.sh/match/dogservice: "{{ .Major }}"
    keel.sh/match/catservice: "{{ .Patch }}"

Also, doesn't the {{ .Minor }} template helpers need a value for the fixed values or can it somehow infer this from the rest of the config during deployment? If not then maybe a format like {{ .Semver 2.x.x }} would be a bit more explicit?

labels:
    keel.sh/match: "{{ .Sem 2.x.x }}" #default
    keel.sh/match/dogservice: "{{ .Sem 1.3.x }}"
    keel.sh/match/catservice: "{{ .Sem 0.9.2-beta }}"

Happy to build this if we can settle on a format

[rusenask]

Hi guys, I have a work-in-progress PR for this #265. Does it make sense? it should cover these scenarios and a bit more.

[krmcbride]

@rusenask Just testing out 0.11.0 for the #265 regex support, but it looks like it doesn't support annotations, so regex syntax is being killed by the k8s labels requirements like you mention in #223 (comment)

Is there a reason to use labels at all for something like this? Just curious.

[rusenask]

@krmcbride, no reason except backwards compatibility. I think we can read annotations first and if policy is not there, then check labels.

[rusenask]

available from 0.13.0-rc1