Update helm chart for cilium-operator to implement per-provider opera… #11837

seanmwinn · 2020-06-02T19:05:50Z

…tor deployment for generic, aws and azure

Signed-off-by: Sean Winn sean@isovalent.com

cilium-operator now packages a separate container for generic use cases, as well as additional container images for Microsoft Azure and Amazon Web Services. The default configuration will deploy the cilium-operator-generic image. For AWS, configure `.Values.global.eni=true` to deploy the cilium-operator-aws image. For Azure configure `.Values.global.azure.enabled=true` to deploy the cilium-operator-azure image. Only one cloud provider is supported in a Cilium deployment.

maintainer-s-little-helper · 2020-06-02T19:05:53Z

Please set the appropriate release note label.

coveralls · 2020-06-02T20:08:10Z

Coverage decreased (-0.02%) to 37.027% when pulling 656a952 on seanmwinn:cilium-11800 into 8a02976 on cilium:master.

aanm

@seanmwinn please also update the release process to have the imagePullPolicy and tag modified.

cilium/Documentation/contributing/release/stable.rst

Lines 42 to 51 in 6d52ae4

    
           #. Update the ``VERSION`` file to represent ``X.Y.Z+1`` 
        
           #. If this is the first release after creating a new release branch. Adjust the 
        
              image pull policy for all ``.sed`` files in ``install/kubernetes/cilium/values.yaml`` from 
        
              ``Always`` to ``IfNotPresent``. 
        
           #. Update Helm chart documentation 
        
              #. Update ``version`` and ``appVersion`` in ``install/kubernetes/cilium/Chart.yaml`` 
        
              #. Update version tag in ``install/kubernetes/cilium/values.yaml`` 
        
           #. Update the image tag versions in the examples:

install/kubernetes/cilium/charts/operator/values.yaml

joestringer · 2020-06-02T22:56:27Z

Marked as release-note/minor because the release note in the PR description provides instructions to users, so it's clearly user-facing.

install/kubernetes/cilium/charts/operator/templates/deployment.yaml

errordeveloper

LGTM overall! Thanks @seanmwinn!

aanm

@joestringer the release process guide needs to be updated, right?

joestringer · 2020-06-03T22:08:09Z

@aanm which step changes?

The makefile change I pushed expands the existing make -C install/kubernetes main target to also fix up the versions in this target, I didn't create separate targets or anything. So AFAICT there's no change to the release process.

aanm · 2020-06-04T07:23:14Z

@aanm which step changes?

The makefile change I pushed expands the existing make -C install/kubernetes main target to also fix up the versions in this target, I didn't create separate targets or anything. So AFAICT there's no change to the release process.

@joestringer step 5 https://docs.cilium.io/en/latest/contributing/release/stable/

joestringer · 2020-06-04T18:24:35Z

@aanm oh right. Yes, we can drop steps 5 & 6 as they are achieved by step 7. That's a separate issue from this PR though, I can send a PR.

joestringer · 2020-06-04T18:27:44Z

@aanm I posted #11898 to address that concern. Do you have any remaining reservations about this PR?

aanm · 2020-06-05T08:31:45Z

test-me-please

aanm · 2020-06-08T15:28:02Z

the CI seem to have failed because the proposed changes https://jenkins.cilium.io/job/Cilium-PR-K8s-newest-kernel-4.9/592/testReport/junit/Suite-k8s-1/18/K8sChaosTest_Connectivity_demo_application_Endpoint_can_still_connect_while_Cilium_is_not_running

aanm · 2020-06-08T16:22:36Z

test-me-please

joestringer · 2020-06-08T19:58:31Z

test-me-please

joestringer · 2020-06-08T23:26:50Z

Looks like the templating is wrong somewhere here, see 139.178.68.33/cilium/operator-generic-generic:latest:
https://jenkins.cilium.io/job/Cilium-PR-Ginkgo-Tests-Kernel/1965/testReport/junit/Suite-k8s-1/17/K8sUpdates_Tests_upgrade_and_downgrade_from_a_Cilium_stable_image_to_master/

	   Warning  Failed     9m26s (x4 over 10m)   kubelet, k8s2      Failed to pull image "139.178.68.33/cilium/operator-generic-generic:latest": rpc error: code = Unknown desc = Error response from daemon: manifest for 139.178.68.33/cilium/operator-generic-generic:latest not found: manifest unknown: manifest unknown

install/kubernetes/cilium/charts/operator/templates/deployment.yaml

install/kubernetes/cilium/charts/operator/values.yaml

test/helpers/kubectl.go

In particular, add the new operator values file to the list of files that we fix these values up for in the make target. Signed-off-by: Joe Stringer <joe@cilium.io>

Update the helm charts for for generic, aws and azure operator images. Fixes: #11800 Signed-off-by: Sean Winn <sean@isovalent.com> Signed-off-by: Joe Stringer <joe@cilium.io>

Documentation/contributing/testing/e2e.rst

jenkinsfiles/ginkgo-eks.Jenkinsfile

Documentation/contributing/testing/e2e.rst

jenkinsfiles/ginkgo-eks.Jenkinsfile

Signed-off-by: Sean Winn <sean@isovalent.com>

joestringer · 2020-06-10T22:11:25Z

test-focus K8sUpdates

joestringer · 2020-06-10T22:45:18Z

Updates test is already providing some feedback:

https://jenkins.cilium.io/job/Cilium-PR-Ginkgo-Tests-Validated-Focus/259/execution/node/175/log/

15:33:21  Expected command: helm install cilium cilium/cilium --version=1.7-dev --namespace=kube-system  --set operator.image.repository=operator  --set preflight.image=cilium-dev  --set global.ipv6.enabled=true  --set global.logSystemLoad=true  --set global.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].values[0]=k8s3  --set global.hubble.enabled=true  --set global.pprof.enabled=true  --set global.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].key=cilium.io/ci-node  --set global.k8sServicePort=6443  --set global.kubeProxyReplacement=strict  --set global.registry=docker.io/cilium  --set global.nodePort.device=enp0s8  --set global.k8s.requireIPv4PodCIDR=true  --set global.cnpStatusUpdates.enabled=true  --set global.bpf.preallocateMaps=true  --set global.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchExpressions[0].operator=NotIn  --set agent.image=cilium  --set global.hubble.listenAddress=:4244  --set global.ci.kubeCacheMutationDetector=true  --set global.sessionAffinity.enabled=false  --set global.debug.enabled=true  --set global.psp.enabled=true  --set global.k8sServiceHost=192.168.36.11  --set global.bpfMasquerade=true  --set global.hostFirewall=true  --set global.tag=v1.7  --set global.etcd.leaseTTL=30s  --set global.ipv4.enabled=true  
15:33:21  To succeed, but it failed:
15:33:21  Exitcode: 1 
15:33:21  Stdout:
15:33:21  
15:33:21  Stderr:
15:33:21   	 coalesce.go:165: warning: skipped value for image: Not a table.
15:33:21  	 coalesce.go:165: warning: skipped value for image: Not a table.
15:33:21  	 coalesce.go:199: warning: destination for image is a table. Ignoring non-table value operator
15:33:21  	 coalesce.go:165: warning: skipped value for image: Not a table.
15:33:21  	 coalesce.go:165: warning: skipped value for image: Not a table.
15:33:21  	 Error: template: cilium/charts/operator/templates/deployment.yaml:129:27: executing "cilium/charts/operator/templates/deployment.yaml" at <.Values.image>: wrong type for value; expected string; got map[string]interface {}
15:33:21  	 
15:33:21  
15:33:21  === Test Finished at 2020-06-10T22:33:17Z====
15:33:21  22:33:17 STEP: Running JustAfterEach block for EntireTestsuite K8sUpdates
15:33:21  ===================== TEST FAILED =====================

Basically the Ginkgo framework is deploying v1.7-dev from helm.cilium.io using a set of Helm options defined in this branch, ie the Helm options correspond to v1.8. This discrepancy means that the image argument handling is inconsistent for the older release. We either need to change the test to use the Cilium-1.7 Helm configuration arguments, or improve our compatibility story in another way.

tgraf · 2020-06-11T11:31:22Z

Given these are doc changes only, we can remove the release blocker flag as we can tag the binary before this is merged.

joestringer · 2020-06-11T17:51:12Z

@tgraf my concern was more the helm changes which we would need to test in all cloud configurations prior to release.

tgraf · 2020-06-11T19:58:51Z

Sure, totally fine to re-add the release blocker label as long as we agree that we don't have to hold rc3 for it.

joestringer

Quick status update to clarify what was merged / what is deferred to resolve in the future.

joestringer · 2020-06-15T17:38:30Z

install/kubernetes/Makefile

-CILIUM_CHARTS := "$(ROOT_DIR)/$(RELATIVE_DIR)/cilium/"
-CILIUM_VALUES := "$(CILIUM_CHARTS)/values.yaml"
+CILIUM_CHARTS := "$(ROOT_DIR)/$(RELATIVE_DIR)/cilium"
+CILIUM_VALUES := "$(CILIUM_CHARTS)/values.yaml" "$(CILIUM_CHARTS)/charts/operator/values.yaml"


When rebasing, this change will still be necessary.

joestringer · 2020-06-15T17:38:52Z

install/kubernetes/cilium/charts/operator/values.yaml

+image:
+  repository: operator
+  tag: latest
+  pullPolicy: Always
+


All of the other changes were already applied in other PRs, except for this change.

(Well, and obviously when this changes the actual daemonset needs to take this into account)

We will need to look at this closely from an upgrade instructions angle because changing the "image" value from a string to a map may cause helm to reject the upgrade.

seanmwinn requested a review from a team as a code owner June 2, 2020 19:05

seanmwinn requested a review from a team June 2, 2020 19:05

maintainer-s-little-helper bot added the dont-merge/needs-release-note label Jun 2, 2020

maintainer-s-little-helper bot added this to In progress in 1.8.0 Jun 2, 2020

aanm requested changes Jun 2, 2020

View reviewed changes

install/kubernetes/cilium/charts/operator/values.yaml Outdated Show resolved Hide resolved

install/kubernetes/cilium/charts/operator/values.yaml Show resolved Hide resolved

aanm added needs-backport/1.8 release-note/misc This PR makes changes that have no direct user impact. labels Jun 2, 2020

maintainer-s-little-helper bot removed the dont-merge/needs-release-note label Jun 2, 2020

maintainer-s-little-helper bot added this to Needs backport from master in 1.8.0 Jun 2, 2020

joestringer requested a review from aanm June 2, 2020 22:54

joestringer added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. and removed release-note/misc This PR makes changes that have no direct user impact. labels Jun 2, 2020

errordeveloper reviewed Jun 3, 2020

View reviewed changes

install/kubernetes/cilium/charts/operator/templates/deployment.yaml Show resolved Hide resolved

errordeveloper approved these changes Jun 3, 2020

View reviewed changes

aanm requested changes Jun 3, 2020

View reviewed changes

joestringer mentioned this pull request Jun 3, 2020

Implement values for hubble-relay to properly control sub chart values #11757

Merged

aanm approved these changes Jun 5, 2020

View reviewed changes

seanmwinn requested review from a team as code owners June 8, 2020 16:03

sayboras reviewed Jun 9, 2020

View reviewed changes

install/kubernetes/cilium/charts/operator/templates/deployment.yaml Show resolved Hide resolved

sayboras reviewed Jun 9, 2020

View reviewed changes

install/kubernetes/cilium/charts/operator/values.yaml Outdated Show resolved Hide resolved

sayboras reviewed Jun 9, 2020

View reviewed changes

test/helpers/kubectl.go Outdated Show resolved Hide resolved

joestringer added the priority/release-blocker label Jun 9, 2020

install: Fix up version/pullPolicy for multiple values files

d6060e6

In particular, add the new operator values file to the list of files that we fix these values up for in the make target. Signed-off-by: Joe Stringer <joe@cilium.io>

seanmwinn requested a review from a team as a code owner June 9, 2020 22:26

Implement per-provider operator deployments

ab55205

Update the helm charts for for generic, aws and azure operator images. Fixes: #11800 Signed-off-by: Sean Winn <sean@isovalent.com> Signed-off-by: Joe Stringer <joe@cilium.io>

seanmwinn requested a review from a team as a code owner June 10, 2020 02:46

aanm requested changes Jun 10, 2020

View reviewed changes

Documentation/contributing/testing/e2e.rst Show resolved Hide resolved

jenkinsfiles/ginkgo-eks.Jenkinsfile Show resolved Hide resolved

aanm requested changes Jun 10, 2020

View reviewed changes

Documentation/contributing/testing/e2e.rst Outdated Show resolved Hide resolved

jenkinsfiles/ginkgo-eks.Jenkinsfile Outdated Show resolved Hide resolved

fix rendering for image names with a / to not append suffixes

656a952

Signed-off-by: Sean Winn <sean@isovalent.com>

tgraf removed the priority/release-blocker label Jun 11, 2020

joestringer mentioned this pull request Jun 11, 2020

Implement per-provider operator deployments in Helm #12029

Merged

aanm added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 12, 2020

joestringer requested changes Jun 15, 2020

View reviewed changes

joestringer removed the needs-backport/1.8 label Jun 17, 2020

maintainer-s-little-helper bot removed this from Needs backport from master in 1.8.0 Jun 17, 2020

seanmwinn closed this Jul 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update helm chart for cilium-operator to implement per-provider opera… #11837

Update helm chart for cilium-operator to implement per-provider opera… #11837

seanmwinn commented Jun 2, 2020 •

edited by errordeveloper

maintainer-s-little-helper bot commented Jun 2, 2020

coveralls commented Jun 2, 2020 •

edited

aanm left a comment

joestringer commented Jun 2, 2020

errordeveloper left a comment

aanm left a comment

joestringer commented Jun 3, 2020 •

edited

aanm commented Jun 4, 2020

joestringer commented Jun 4, 2020

joestringer commented Jun 4, 2020

aanm commented Jun 5, 2020

aanm commented Jun 8, 2020

aanm commented Jun 8, 2020

joestringer commented Jun 8, 2020

joestringer commented Jun 8, 2020

joestringer commented Jun 10, 2020

joestringer commented Jun 10, 2020 •

edited

tgraf commented Jun 11, 2020

joestringer commented Jun 11, 2020

tgraf commented Jun 11, 2020

joestringer left a comment

joestringer Jun 15, 2020

joestringer Jun 15, 2020

joestringer Jun 15, 2020

joestringer Jun 15, 2020

	#. Update the ``VERSION`` file to represent ``X.Y.Z+1``
	#. If this is the first release after creating a new release branch. Adjust the
	image pull policy for all ``.sed`` files in ``install/kubernetes/cilium/values.yaml`` from
	``Always`` to ``IfNotPresent``.
	#. Update Helm chart documentation

	#. Update ``version`` and ``appVersion`` in ``install/kubernetes/cilium/Chart.yaml``
	#. Update version tag in ``install/kubernetes/cilium/values.yaml``

	#. Update the image tag versions in the examples:

Update helm chart for cilium-operator to implement per-provider opera… #11837

Update helm chart for cilium-operator to implement per-provider opera… #11837

Conversation

seanmwinn commented Jun 2, 2020 • edited by errordeveloper

maintainer-s-little-helper bot commented Jun 2, 2020

coveralls commented Jun 2, 2020 • edited

aanm left a comment

Choose a reason for hiding this comment

joestringer commented Jun 2, 2020

errordeveloper left a comment

Choose a reason for hiding this comment

aanm left a comment

Choose a reason for hiding this comment

joestringer commented Jun 3, 2020 • edited

aanm commented Jun 4, 2020

joestringer commented Jun 4, 2020

joestringer commented Jun 4, 2020

aanm commented Jun 5, 2020

aanm commented Jun 8, 2020

aanm commented Jun 8, 2020

joestringer commented Jun 8, 2020

joestringer commented Jun 8, 2020

joestringer commented Jun 10, 2020

joestringer commented Jun 10, 2020 • edited

tgraf commented Jun 11, 2020

joestringer commented Jun 11, 2020

tgraf commented Jun 11, 2020

joestringer left a comment

Choose a reason for hiding this comment

joestringer Jun 15, 2020

Choose a reason for hiding this comment

joestringer Jun 15, 2020

Choose a reason for hiding this comment

joestringer Jun 15, 2020

Choose a reason for hiding this comment

joestringer Jun 15, 2020

Choose a reason for hiding this comment

seanmwinn commented Jun 2, 2020 •

edited by errordeveloper

coveralls commented Jun 2, 2020 •

edited

joestringer commented Jun 3, 2020 •

edited

joestringer commented Jun 10, 2020 •

edited