-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Added SMI canary deployment strategy to KubernetesManifest task #11218
Conversation
Tasks/KubernetesManifestV0/Strings/resources.resjson/en-US/resources.resjson
Outdated
Show resolved
Hide resolved
Tests are failing. Could you check if the expected behavior for default ones are changing? |
Given that this logic works only if any service mesh is enabled on the cluster, shouldn't you add a check if it exists on the cluster? |
@@ -11,7 +11,10 @@ | |||
"loc.input.help.namespace": "Sets the namespace for the commands by using the –namespace flag. If the namespace is not provided, the commands will run in the default namespace.", | |||
"loc.input.label.strategy": "Strategy", | |||
"loc.input.help.strategy": "Deployment strategy to be used", | |||
"loc.input.label.trafficSplitMethod": "Traffic split method", | |||
"loc.input.help.trafficSplitMethod": "Deployment strategy to be used", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Label and help are looking like related. Can we add some more details in 'Help'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i will check with PM and will add.
Tasks/KubernetesManifestV0/Strings/resources.resjson/en-US/resources.resjson
Outdated
Show resolved
Hide resolved
dbb4f20
to
e6ffc58
Compare
"apiVersion": "split.smi-spec.io/v1alpha1", | ||
"kind": "TrafficSplit", | ||
"metadata": { | ||
"name": "%s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could directly use ` and replace it here instead of using formatter
// In case of SMI traffic split strategy when deployment is promoted, first we will redirect traffic to | ||
// Canary deployment, then update stable deployment and then redirect traffic to stable deployment | ||
tl.debug('Redirecting traffic to canary deployment'); | ||
SMICanaryDeploymentHelper.redirectTrafficToCanaryDeployment(kubectl, TaskInputParameters.manifests); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just want to confirm, Redirecting entire traffic to canary during promote, how are we making sure Canary pods can handle that much traffic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In SMI, first you will redirect traffic to other deploymnt(canary) before updating stable. Generally customer will increase the canary pods before updating stable one.
9c0a66b
to
09ca9a4
Compare
Added SMI canary deployment strategy to KubernetesManifest task