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

rollout support patchPodTemplateMetadata #157

Merged
merged 1 commit into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/v1alpha1/batchrelease_plan_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ type ReleasePlan struct {
// FinalizingPolicy define the behavior of controller when phase enter Finalizing
// Defaults to "Immediate"
FinalizingPolicy FinalizingPolicyType `json:"finalizingPolicy,omitempty"`
// PatchPodTemplateMetadata indicates patch configuration(e.g. labels, annotations) to the canary deployment podTemplateSpec.metadata
// only support for canary deployment
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is support for cloneset possible?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still depends on the scenario, there is no canary workload for cloneSet, so it may be necessary to look at the scenario later

// +optional
PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"`
}

type FinalizingPolicyType string
Expand Down
11 changes: 11 additions & 0 deletions api/v1alpha1/rollout_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,17 @@ type CanaryStrategy struct {
// FailureThreshold.
// Defaults to nil.
FailureThreshold *intstr.IntOrString `json:"failureThreshold,omitempty"`
// PatchPodTemplateMetadata indicates patch configuration(e.g. labels, annotations) to the canary deployment podTemplateSpec.metadata
// only support for canary deployment
// +optional
PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"`
}

type PatchPodTemplateMetadata struct {
// annotations
Annotations map[string]string `json:"annotations,omitempty"`
// labels
Labels map[string]string `json:"labels,omitempty"`
}

// CanaryStep defines a step of a canary workload.
Expand Down
39 changes: 39 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions config/crd/bases/rollouts.kruise.io_batchreleases.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,22 @@ spec:
description: FinalizingPolicy define the behavior of controller
when phase enter Finalizing Defaults to "Immediate"
type: string
patchPodTemplateMetadata:
description: PatchPodTemplateMetadata indicates patch configuration(e.g.
labels, annotations) to the canary deployment podTemplateSpec.metadata
only support for canary deployment
properties:
annotations:
additionalProperties:
type: string
description: annotations
type: object
labels:
additionalProperties:
type: string
description: labels
type: object
type: object
rolloutID:
description: RolloutID indicates an id for each rollout progress
type: string
Expand Down
16 changes: 16 additions & 0 deletions config/crd/bases/rollouts.kruise.io_rollouts.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,22 @@ spec:
is nil, Rollout will use the MaxUnavailable of workload
as its FailureThreshold. Defaults to nil.
x-kubernetes-int-or-string: true
patchPodTemplateMetadata:
description: PatchPodTemplateMetadata indicates patch configuration(e.g.
labels, annotations) to the canary deployment podTemplateSpec.metadata
only support for canary deployment
properties:
annotations:
additionalProperties:
type: string
description: annotations
type: object
labels:
additionalProperties:
type: string
description: labels
type: object
type: object
steps:
description: Steps define the order of phases to execute release
in batches(20%, 40%, 60%, 80%, 100%)
Expand Down
5 changes: 5 additions & 0 deletions lua_configuration/trafficrouting_ingress/mse.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ if ( obj.weight ~= "-1" )
then
annotations["nginx.ingress.kubernetes.io/canary-weight"] = obj.weight
end
if ( annotations["mse.ingress.kubernetes.io/service-subset"] )
then
annotations["mse.ingress.kubernetes.io/service-subset"] = "gray"
end

if ( obj.requestHeaderModifier )
then
local str = ''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,19 @@ func (r *realCanaryController) create(release *v1alpha1.BatchRelease, template *

// spec
canary.Spec = *template.Spec.DeepCopy()
// todo, patch canary pod metadata
// patch canary pod metadata
if release.Spec.ReleasePlan.PatchPodTemplateMetadata != nil {
patch := release.Spec.ReleasePlan.PatchPodTemplateMetadata
for k, v := range patch.Labels {
canary.Spec.Template.Labels[k] = v
}
if canary.Spec.Template.Annotations == nil {
canary.Spec.Template.Annotations = map[string]string{}
}
for k, v := range patch.Annotations {
canary.Spec.Template.Annotations[k] = v
veophi marked this conversation as resolved.
Show resolved Hide resolved
}
}
canary.Spec.Replicas = pointer.Int32Ptr(0)
canary.Spec.Paused = false

Expand Down Expand Up @@ -169,7 +181,7 @@ func (r *realCanaryController) listDeployment(release *v1alpha1.BatchRelease, op
}

// return the latest deployment with the newer creation time
func filterCanaryDeployment(ds []*apps.Deployment, template *corev1.PodTemplateSpec) *apps.Deployment {
func filterCanaryDeployment(release *v1alpha1.BatchRelease, ds []*apps.Deployment, template *corev1.PodTemplateSpec) *apps.Deployment {
if len(ds) == 0 {
return nil
}
Expand All @@ -179,9 +191,18 @@ func filterCanaryDeployment(ds []*apps.Deployment, template *corev1.PodTemplateS
if template == nil {
return ds[0]
}
var ignoreLabels, ignoreAnno = make([]string, 0), make([]string, 0)
if release.Spec.ReleasePlan.PatchPodTemplateMetadata != nil {
patch := release.Spec.ReleasePlan.PatchPodTemplateMetadata
for k := range patch.Labels {
ignoreLabels = append(ignoreLabels, k)
}
for k := range patch.Annotations {
ignoreAnno = append(ignoreAnno, k)
}
}
for _, d := range ds {
// todo, remove the canary pod metadata
if util.EqualIgnoreHash(template, &d.Spec.Template) {
if util.EqualIgnoreSpecifyMetadata(template, &d.Spec.Template, ignoreLabels, ignoreAnno) {
return d
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (rc *realController) BuildCanaryController(release *v1alpha1.BatchRelease)
if client.IgnoreNotFound(err) != nil {
return rc, err
}
rc.canaryObject = filterCanaryDeployment(util.FilterActiveDeployment(ds), template)
rc.canaryObject = filterCanaryDeployment(release, util.FilterActiveDeployment(ds), template)
if rc.canaryObject == nil {
return rc, control.GenerateNotFoundError(fmt.Sprintf("%v-canary", rc.stableKey), "Deployment")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ func getCanaryDeployment(release *v1alpha1.BatchRelease, stable *apps.Deployment
if len(ds) == 0 {
return nil
}
return filterCanaryDeployment(ds, &stable.Spec.Template)
return filterCanaryDeployment(release, ds, &stable.Spec.Template)
}

func checkWorkloadInfo(stableInfo *util.WorkloadInfo, deployment *apps.Deployment) {
Expand Down
10 changes: 5 additions & 5 deletions pkg/controller/rollout/rollout_canary.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,11 +361,11 @@ func createBatchRelease(rollout *v1alpha1.Rollout, rolloutID string, batch int32
},
},
ReleasePlan: v1alpha1.ReleasePlan{
Batches: batches,
RolloutID: rolloutID,
BatchPartition: utilpointer.Int32Ptr(batch),
FailureThreshold: rollout.Spec.Strategy.Canary.FailureThreshold,
// PatchPodTemplateMetadata: rollout.Spec.Strategy.Canary.PatchPodTemplateMetadata,
Batches: batches,
RolloutID: rolloutID,
BatchPartition: utilpointer.Int32Ptr(batch),
FailureThreshold: rollout.Spec.Strategy.Canary.FailureThreshold,
PatchPodTemplateMetadata: rollout.Spec.Strategy.Canary.PatchPodTemplateMetadata,
},
},
}
Expand Down
16 changes: 10 additions & 6 deletions pkg/trafficrouting/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,17 @@ func (m *Manager) FinalisingTrafficRouting(c *TrafficRoutingContext, onlyRestore
if err = trController.Finalise(context.TODO()); err != nil {
return false, err
}
// remove canary service
err = m.Delete(context.TODO(), cService)
if err != nil && !errors.IsNotFound(err) {
klog.Errorf("%s remove canary service(%s) failed: %s", c.Key, cService.Name, err.Error())
return false, err
// end to end deployment, don't remove the canary service;
// because canary service is stable service
if !c.OnlyTrafficRouting {
// remove canary service
err = m.Delete(context.TODO(), cService)
if err != nil && !errors.IsNotFound(err) {
klog.Errorf("%s remove canary service(%s) failed: %s", c.Key, cService.Name, err.Error())
return false, err
}
klog.Infof("%s remove canary service(%s) success", c.Key, cService.Name)
}
klog.Infof("%s remove canary service(%s) success", c.Key, cService.Name)
return true, nil
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/trafficrouting/network/ingress/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ var (
then
annotations["nginx.ingress.kubernetes.io/canary-weight"] = obj.weight
end
if ( annotations["mse.ingress.kubernetes.io/service-subset"] )
then
annotations["mse.ingress.kubernetes.io/service-subset"] = "gray"
end
if ( obj.requestHeaderModifier )
then
local str = ''
Expand Down Expand Up @@ -317,6 +321,7 @@ func TestEnsureRoutes(t *testing.T) {
canary.Name = "echoserver-canary"
canary.Annotations["nginx.ingress.kubernetes.io/canary"] = "true"
canary.Annotations["nginx.ingress.kubernetes.io/canary-weight"] = "0"
canary.Annotations["mse.ingress.kubernetes.io/service-subset"] = ""
canary.Spec.Rules[0].HTTP.Paths = canary.Spec.Rules[0].HTTP.Paths[:1]
canary.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary"
canary.Spec.Rules[1].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary"
Expand Down Expand Up @@ -369,6 +374,7 @@ func TestEnsureRoutes(t *testing.T) {
expect.Annotations["nginx.ingress.kubernetes.io/canary-by-header"] = "user_id"
expect.Annotations["nginx.ingress.kubernetes.io/canary-by-header-value"] = "123456"
expect.Annotations["mse.ingress.kubernetes.io/request-header-control-update"] = "gray blue\ngray green\n"
expect.Annotations["mse.ingress.kubernetes.io/service-subset"] = "gray"
expect.Spec.Rules[0].HTTP.Paths = expect.Spec.Rules[0].HTTP.Paths[:1]
expect.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary"
expect.Spec.Rules[1].HTTP.Paths[0].Backend.Service.Name = "echoserver-canary"
Expand Down
3 changes: 0 additions & 3 deletions pkg/util/controller_finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ type Workload struct {
PodTemplateHash string
// Revision hash key
RevisionLabelKey string
// label selector
Selector *metav1.LabelSelector

// Is it in rollback phase
IsInRollback bool
Expand Down Expand Up @@ -311,7 +309,6 @@ func (r *ControllerFinder) getDeployment(namespace string, ref *rolloutv1alpha1.
StableRevision: stableRs.Labels[apps.DefaultDeploymentUniqueLabelKey],
CanaryRevision: ComputeHash(&stable.Spec.Template, nil),
RevisionLabelKey: apps.DefaultDeploymentUniqueLabelKey,
Selector: stable.Spec.Selector,
}

// not in rollout progressing
Expand Down
19 changes: 19 additions & 0 deletions pkg/util/workloads_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,25 @@ func SafeEncodeString(s string) string {
return string(r)
}

func EqualIgnoreSpecifyMetadata(template1, template2 *v1.PodTemplateSpec, ignoreLabels, ignoreAnno []string) bool {
t1Copy := template1.DeepCopy()
t2Copy := template2.DeepCopy()
if ignoreLabels == nil {
ignoreLabels = make([]string, 0)
}
// default remove the hash label
ignoreLabels = append(ignoreLabels, apps.DefaultDeploymentUniqueLabelKey)
for _, k := range ignoreLabels {
delete(t1Copy.Labels, k)
delete(t2Copy.Labels, k)
}
for _, k := range ignoreAnno {
delete(t1Copy.Annotations, k)
delete(t2Copy.Annotations, k)
}
return apiequality.Semantic.DeepEqual(t1Copy, t2Copy)
}

// EqualIgnoreHash compare template without pod-template-hash label
func EqualIgnoreHash(template1, template2 *v1.PodTemplateSpec) bool {
t1Copy := template1.DeepCopy()
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/rollout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,10 @@ var _ = SIGDescribe("Rollout", func() {
},
},
}
rollout.Spec.Strategy.Canary.PatchPodTemplateMetadata = &v1alpha1.PatchPodTemplateMetadata{
Labels: map[string]string{"pod": "canary"},
Annotations: map[string]string{"pod": "canary"},
}
CreateObject(rollout)

By("Creating workload and waiting for all pods ready...")
Expand All @@ -457,6 +461,10 @@ var _ = SIGDescribe("Rollout", func() {
workload := &apps.Deployment{}
Expect(ReadYamlToObject("./test_data/rollout/deployment.yaml", workload)).ToNot(HaveOccurred())
workload.Spec.Replicas = utilpointer.Int32(3)
workload.Spec.Template.Labels["pod"] = "stable"
workload.Spec.Template.Annotations = map[string]string{
"pod": "stable",
}
CreateObject(workload)
WaitDeploymentAllPodsReady(workload)

Expand Down Expand Up @@ -484,6 +492,10 @@ var _ = SIGDescribe("Rollout", func() {
cIngress := &netv1.Ingress{}
Expect(GetObject(service.Name+"-canary", cIngress)).NotTo(HaveOccurred())
Expect(cIngress.Annotations[fmt.Sprintf("%s/canary-weight", nginxIngressAnnotationDefaultPrefix)]).Should(Equal("20"))
canaryWorkload, err := GetCanaryDeployment(workload)
Expect(err).NotTo(HaveOccurred())
Expect(canaryWorkload.Spec.Template.Annotations["pod"]).Should(Equal("canary"))
Expect(canaryWorkload.Spec.Template.Labels["pod"]).Should(Equal("canary"))

// resume rollout canary
ResumeRolloutCanary(rollout.Name)
Expand Down