Page MenuHomePhabricator
Create Task
Maniphest T303279

Fix calico, cfssl-issuer and knative-serving Helm dependencies
Closed, ResolvedPublic
Actions

Description

We currently have a nice typo, dependecies (missing n) in various Helm charts: knative-serving, calico and cfssl-issuer

I tried to fix them via https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/768800, but the CI and helmfile diff look like all the CRDs of the related -crds charts are going to be re-added. We are currently relying on the dependency between the charts brought by hemfile, so we may not need to have an explicit Helm dependency as well.

The above change was reverted in https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/768800 to figure out what's best to do.

Details

ProjectBranchLines +/-Subject
operations/deployment-chartsmaster+11 -5kserve-inference: fix dependencies in Chart.yaml
operations/deployment-chartsmaster+24 -20calico, cfssl-issuer: Remove chart defined dependencies
operations/deployment-chartsmaster+11 -5knative-serving: improve chart's dependencies
operations/deployment-chartsmaster+72 -11Update calico to v3.23.3
operations/deployment-chartsmaster+3 -3calico,cfssl-issuer,knative-serving: bump chart's version
Customize query in gerrit

Related Objects
Search...

Event Timeline

elukey created this task.Mar 8 2022, 3:29 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 8 2022, 3:29 PM
gerritbot added a comment.Mar 8 2022, 3:40 PM

Change 769062 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] calico,cfssl-issuer,knative-serving: bump chart's version

https://gerrit.wikimedia.org/r/769062

gerritbot added a project: Patch-For-Review.Mar 8 2022, 3:40 PM
gerritbot added a comment.Mar 8 2022, 3:54 PM

Change 769062 merged by Elukey:

[operations/deployment-charts@master] calico,cfssl-issuer,knative-serving: bump chart's version

https://gerrit.wikimedia.org/r/769062

Maintenance_bot removed a project: Patch-For-Review.Mar 8 2022, 4:10 PM
elukey moved this task from Unsorted to Backlog/SRE on the Machine-Learning-Team board.Apr 26 2022, 7:27 AM
JMeybohm added a parent task: T307943: Update Kubernetes clusters to v1.23.Jun 9 2022, 9:46 AM
gerritbot added a comment.Aug 26 2022, 10:15 AM

Change 826810 had a related patch set uploaded (by JMeybohm; author: JMeybohm):

[operations/deployment-charts@master] Update calico to v3.23.3

https://gerrit.wikimedia.org/r/826810

gerritbot added a project: Patch-For-Review.Aug 26 2022, 10:15 AM
JMeybohm mentioned this in T316347: Helm chart packaging should update dependencies.Aug 26 2022, 1:01 PM
jijiki moved this task from Incoming 🐫 to 🙈🙉🙊Backlog on the serviceops board.Sep 28 2022, 2:19 PM
JMeybohm added a comment.Oct 4 2022, 8:38 AM

Fixed for calico with v3.23.3

gerritbot added a comment.Oct 4 2022, 8:39 AM

Change 826810 merged by jenkins-bot:

[operations/deployment-charts@master] Update calico to v3.23.3

https://gerrit.wikimedia.org/r/826810

Maintenance_bot removed a project: Patch-For-Review.Oct 4 2022, 9:31 AM
jijiki moved this task from 🙈🙉🙊Backlog to Backlog FY22-23 🚜 on the serviceops board.Nov 8 2022, 6:12 PM
jijiki moved this task from Backlog FY22-23 🚜 to ⎈Kubernetes on the serviceops board.Nov 9 2022, 2:03 PM
JMeybohm added a comment.Nov 10 2022, 10:21 AM

Fixed for cfss-issuer in chart version 0.3.0

JMeybohm triaged this task as Low priority.Nov 18 2022, 9:20 AM
gerritbot added a comment.Nov 28 2022, 3:38 PM

Change 861399 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] knative-serving: improve chart's dependencies

https://gerrit.wikimedia.org/r/861399

gerritbot added a project: Patch-For-Review.Nov 28 2022, 3:38 PM
JMeybohm added a comment.Nov 29 2022, 4:36 PM

When deploying the updated calico chart to my test cluster I realized that the fixed dependency to the CRD chart does not mean that chart is installed (e.g. CRDs available) before the main chart is installed (obviously the deployment failed). In helmfile.d we deploy the CRD chart as a release before deploying the main chart. This unfortunately does no longer work as helm detects the CRDs already being present (from a different release) and fails to deploy.

This is very in line with https://helm.sh/docs/chart_best_practices/custom_resource_definitions/ and it seems there is no proper way out. Including the CRDs as non-templated files inin the crd/ folder of the main chart is not a real option as there is no support for updating those CRDs.

Speaking with @elukey about this we decided that the easiest way forward is probably to stick to not declaring the dependency to CRD charts in the main charts (which is basically what we did with the typo) and maintaining their relationship via helmfile (see https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/861399).

I'll undo/unfix the cfssl and calico charts.

JMeybohm added subscribers: bking, BTullis.Nov 29 2022, 5:24 PM

CC @BTullis & @bking: This might be relevant for operators as well.

BTullis added a comment.Nov 29 2022, 5:35 PM

Thanks @JMeybohm - I'll definitely bear that in mind.

From my work so far with the spark-operator it seems that the operator itself deploys the CRDs required for subsequent creation of SparkApplication and ScheduledSparkApplication resources.
As in, none of these objects are created at the time of the helm chart being deployed.

I'll keep my eyes open to any problems of this type though and let you know.

gerritbot added a comment.Nov 30 2022, 10:45 AM

Change 861399 merged by Elukey:

[operations/deployment-charts@master] knative-serving: improve chart's dependencies

https://gerrit.wikimedia.org/r/861399

Maintenance_bot removed a project: Patch-For-Review.Nov 30 2022, 11:32 AM
gerritbot added a comment.Dec 13 2022, 9:58 AM

Change 867529 had a related patch set uploaded (by JMeybohm; author: JMeybohm):

[operations/deployment-charts@master] calico, cfssl-issuer: Remove chart defined dependencies

https://gerrit.wikimedia.org/r/867529

gerritbot added a project: Patch-For-Review.Dec 13 2022, 9:58 AM
JMeybohm closed this task as Resolved.Dec 13 2022, 2:03 PM
JMeybohm claimed this task.
gerritbot added a comment.Dec 13 2022, 2:05 PM

Change 867529 merged by jenkins-bot:

[operations/deployment-charts@master] calico, cfssl-issuer: Remove chart defined dependencies

https://gerrit.wikimedia.org/r/867529

gerritbot added a comment.Dec 13 2022, 2:30 PM

Change 867600 had a related patch set uploaded (by Elukey; author: Elukey):

[operations/deployment-charts@master] kserve-inference: fix dependencies in Chart.yaml

https://gerrit.wikimedia.org/r/867600

gerritbot added a comment.Dec 14 2022, 2:22 PM

Change 867600 merged by Elukey:

[operations/deployment-charts@master] kserve-inference: fix dependencies in Chart.yaml

https://gerrit.wikimedia.org/r/867600

Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL