update manifests for dev releases #9754
Conversation
marvin-tigera
added
release-note-required
radTuti
added
docs-not-required
marvin-tigera
removed
release-note-required
| --set tigeraOperator.image=$OPERATOR_IMAGE \ | ||
| --set tigeraOperator.version=$OPERATOR_VERSION \ | ||
| --set tigeraOperator.registry=$OPERATOR_REGISTRY \ | ||
| --set calicoctl.image=$REGISTRY/ctl \ |
There was a problem hiding this comment.
Seems wrong that calicoctl.image is of the form REGISTRY/CTL whereas the tigeraOperator.image is just of the form tigera/operator with the registry configured separately - we should be using consistent meanings for image / version / registry.
There was a problem hiding this comment.
the calicoctl images are pushed as /ctl
sadly we currently do not follow a consistent pattern b/w operator and monorepo images, that will require a bigger overhaul which is beyond the scope of this PR
There was a problem hiding this comment.
The calicoctl images are pushed as /ctl
Surely they must be pushed as docker.io/calico/ctl no?
Operator has two separate config options - registry and image
Calicoctl has a single option - image - that appears to be a merged version of registry and image.
There was a problem hiding this comment.
For my dev images, it is created as docker.io/tuticat/ctl
When the default registry is not overwritten, this be set to calico/ctl which matches the current state of image: calico/ctl:master
| @@ -891,7 +897,7 @@ func (r *CalicoManager) generateManifests() error { | |||
| } | |||
|
|
|||
| func (r *CalicoManager) resetManifests() { | |||
| if _, err := r.runner.RunInDir(r.repoRoot, "git", []string{"checkout", "manifests"}, nil); err != nil { | |||
| if _, err := r.runner.RunInDir(r.repoRoot, "git", []string{"checkout", "manifests", "charts"}, nil); err != nil { | |||
There was a problem hiding this comment.
What change happens in the charts diretory? I can't see where we're modifying that
There was a problem hiding this comment.
the images in charts/calico/values.yaml have their registies updated by manifests/generate.sh here
There was a problem hiding this comment.
Ah, right. I need to think about that one for a moment. I think it's a change in scope of the generate.sh script that I might not be completely comfortable with. That script is meant to deterministically generate manifests based on values.yaml, not be responsible for modifying values.yaml itself.
revert some autoformatting
radTuti
changed the title
Description
The maifests generated when the registry is overwritten are currently incorrect since the generate manifests currently does not update the registries. This attempts to fix that by:
charts/calico/values.yamlRelated issues/PRs
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.