Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase scope of canary deployment revision check #233

Closed
zmiccar opened this issue Jul 4, 2019 · 2 comments · Fixed by #240
Closed

Increase scope of canary deployment revision check #233

zmiccar opened this issue Jul 4, 2019 · 2 comments · Fixed by #240
Labels
kind/enhancement Improvement request for an existing feature

Comments

@zmiccar
Copy link

zmiccar commented Jul 4, 2019

Flagger currently only compares the PodSpec of a deployment when checking for updates. Implementation of the check can be found here: https://github.com/weaveworks/flagger/blob/3da86fe118b5ac2830e066eaf926b9a92f45fa3e/pkg/canary/deployer.go#L135 This is limiting as there are other mutable fields in the DeploymentSpec, such as .PodTemplateSpec.metadata, .replicas, etc.

A suggestion is to compare the whole DeploymentSpec instead so that we are in control of what triggers a new revision.

While discussing this on Slack, @stefanprodan linked to a previous issue that caused the label copying to be removed. I suggest making this configurable instead since the issue described is very specific to jx.

@stefanprodan
Copy link
Member

A remark on the replica count: Flagger needs to manipulate the number of replicas running and I don't think it will ever be included in the diff check. To control the number of replicas you have to use HPA. You set the same value for min and max replicas in the HPA if you don't need auto scaling.

@stefanprodan
Copy link
Member

stefanprodan commented Jul 4, 2019

Including the PodTemplateSpec.metadata in the diff is a backwards incompatible change.

I see two options:

  • implement it the under a flag that can be enabled at install time and switched on by default on the next major release
  • add a field in the Canary spec so this can be enabled on per deployment basis

Regarding the propagation of the pod annotations and labels from canary to primary, this is already happening. The deployment labels and annotations are being skipped not the pod ones.

@stefanprodan stefanprodan added the kind/enhancement Improvement request for an existing feature label Jul 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvement request for an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants