Skip to content

Commit

Permalink
rollout support patchPodTemplateMetadata (#157)
Browse files Browse the repository at this point in the history
Signed-off-by: liheng.zms <[email protected]>
  • Loading branch information
zmberg authored Jul 10, 2023
1 parent 88e4bb7 commit bc014ea
Show file tree
Hide file tree
Showing 15 changed files with 170 additions and 20 deletions.
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
// +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
}
}
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

0 comments on commit bc014ea

Please sign in to comment.