From 0ff23f663673f6b4f4a93d3d2a5143aca7e6bf5a Mon Sep 17 00:00:00 2001 From: myname4423 <57184070+myname4423@users.noreply.github.com> Date: Tue, 12 Mar 2024 16:37:29 +0800 Subject: [PATCH] Allow Not generating Canary Service && Fixed a bug caused by NOT considering case-insensitivity. (#200) * Fixed a bug caused by NOT considering case-insensitivity. Signed-off-by: yunbo * add DisableGenerateCanaryService for CanaryStrategy amend1: update crd yaml amend2: add DisableGenerateCanaryService for v1alpha1 Signed-off-by: yunbo --------- Signed-off-by: yunbo Co-authored-by: yunbo --- api/v1alpha1/conversion.go | 14 ++++++++------ api/v1alpha1/rollout_types.go | 2 ++ api/v1beta1/rollout_types.go | 2 ++ .../bases/rollouts.kruise.io_rollouts.yaml | 8 ++++++++ pkg/controller/rollout/rollout_progressing.go | 19 ++++++++++--------- pkg/trafficrouting/manager.go | 19 ++++++++++++------- .../custom_network_provider.go | 5 +++-- 7 files changed, 45 insertions(+), 24 deletions(-) diff --git a/api/v1alpha1/conversion.go b/api/v1alpha1/conversion.go index bab7259e..5164c1ca 100644 --- a/api/v1alpha1/conversion.go +++ b/api/v1alpha1/conversion.go @@ -19,6 +19,8 @@ package v1alpha1 import ( "fmt" + "strings" + "github.com/openkruise/rollouts/api/v1beta1" "k8s.io/apimachinery/pkg/util/intstr" utilpointer "k8s.io/utils/pointer" @@ -74,7 +76,7 @@ func (src *Rollout) ConvertTo(dst conversion.Hub) error { obj.Spec.Strategy.Canary.PatchPodTemplateMetadata.Labels[k] = v } } - if src.Annotations[RolloutStyleAnnotation] != string(PartitionRollingStyle) { + if !strings.EqualFold(src.Annotations[RolloutStyleAnnotation], string(PartitionRollingStyle)) { obj.Spec.Strategy.Canary.EnableExtraWorkloadForCanary = true } if src.Annotations[TrafficRoutingAnnotation] != "" { @@ -211,9 +213,9 @@ func (dst *Rollout) ConvertFrom(src conversion.Hub) error { dst.Annotations = map[string]string{} } if srcV1beta1.Spec.Strategy.Canary.EnableExtraWorkloadForCanary { - dst.Annotations[RolloutStyleAnnotation] = string(CanaryRollingStyle) + dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(CanaryRollingStyle)) } else { - dst.Annotations[RolloutStyleAnnotation] = string(PartitionRollingStyle) + dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(PartitionRollingStyle)) } if srcV1beta1.Spec.Strategy.Canary.TrafficRoutingRef != "" { dst.Annotations[TrafficRoutingAnnotation] = srcV1beta1.Spec.Strategy.Canary.TrafficRoutingRef @@ -336,7 +338,7 @@ func (src *BatchRelease) ConvertTo(dst conversion.Hub) error { obj.Spec.ReleasePlan.PatchPodTemplateMetadata.Labels[k] = v } } - if src.Annotations[RolloutStyleAnnotation] != string(PartitionRollingStyle) { + if !strings.EqualFold(src.Annotations[RolloutStyleAnnotation], string(PartitionRollingStyle)) { obj.Spec.ReleasePlan.EnableExtraWorkloadForCanary = true } @@ -416,9 +418,9 @@ func (dst *BatchRelease) ConvertFrom(src conversion.Hub) error { dst.Annotations = map[string]string{} } if srcV1beta1.Spec.ReleasePlan.EnableExtraWorkloadForCanary { - dst.Annotations[RolloutStyleAnnotation] = string(CanaryRollingStyle) + dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(CanaryRollingStyle)) } else { - dst.Annotations[RolloutStyleAnnotation] = string(PartitionRollingStyle) + dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(PartitionRollingStyle)) } // status diff --git a/api/v1alpha1/rollout_types.go b/api/v1alpha1/rollout_types.go index 997da5bc..697287d8 100644 --- a/api/v1alpha1/rollout_types.go +++ b/api/v1alpha1/rollout_types.go @@ -121,6 +121,8 @@ type CanaryStrategy struct { // only support for canary deployment // +optional PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"` + // canary service will not be generated if DisableGenerateCanaryService is true + DisableGenerateCanaryService bool `json:"disableGenerateCanaryService,omitempty"` } type PatchPodTemplateMetadata struct { diff --git a/api/v1beta1/rollout_types.go b/api/v1beta1/rollout_types.go index 5287185b..25c91106 100644 --- a/api/v1beta1/rollout_types.go +++ b/api/v1beta1/rollout_types.go @@ -101,6 +101,8 @@ type CanaryStrategy struct { EnableExtraWorkloadForCanary bool `json:"enableExtraWorkloadForCanary,omitempty"` // TrafficRoutingRef is TrafficRouting's Name TrafficRoutingRef string `json:"trafficRoutingRef,omitempty"` + // canary service will not be generated if DisableGenerateCanaryService is true + DisableGenerateCanaryService bool `json:"disableGenerateCanaryService,omitempty"` } type PatchPodTemplateMetadata struct { diff --git a/config/crd/bases/rollouts.kruise.io_rollouts.yaml b/config/crd/bases/rollouts.kruise.io_rollouts.yaml index 226cf5c7..62c072a6 100644 --- a/config/crd/bases/rollouts.kruise.io_rollouts.yaml +++ b/config/crd/bases/rollouts.kruise.io_rollouts.yaml @@ -99,6 +99,10 @@ spec: description: CanaryStrategy defines parameters for a Replica Based Canary properties: + disableGenerateCanaryService: + description: canary service will not be generated if DisableGenerateCanaryService + is true + type: boolean failureThreshold: anyOf: - type: integer @@ -574,6 +578,10 @@ spec: description: CanaryStrategy defines parameters for a Replica Based Canary properties: + disableGenerateCanaryService: + description: canary service will not be generated if DisableGenerateCanaryService + is true + type: boolean enableExtraWorkloadForCanary: description: 'If true, then it will create new deployment for canary, such as: workload-demo-canary. When user verifies diff --git a/pkg/controller/rollout/rollout_progressing.go b/pkg/controller/rollout/rollout_progressing.go index 94905a8e..0aaf5975 100644 --- a/pkg/controller/rollout/rollout_progressing.go +++ b/pkg/controller/rollout/rollout_progressing.go @@ -452,14 +452,15 @@ func newTrafficRoutingContext(c *RolloutContext) *trafficrouting.TrafficRoutingC revisionLabelKey = c.Workload.RevisionLabelKey } return &trafficrouting.TrafficRoutingContext{ - Key: fmt.Sprintf("Rollout(%s/%s)", c.Rollout.Namespace, c.Rollout.Name), - Namespace: c.Rollout.Namespace, - ObjectRef: c.Rollout.Spec.Strategy.Canary.TrafficRoutings, - Strategy: currentStep.TrafficRoutingStrategy, - OwnerRef: *metav1.NewControllerRef(c.Rollout, rolloutControllerKind), - RevisionLabelKey: revisionLabelKey, - StableRevision: c.NewStatus.CanaryStatus.StableRevision, - CanaryRevision: c.NewStatus.CanaryStatus.PodTemplateHash, - LastUpdateTime: c.NewStatus.CanaryStatus.LastUpdateTime, + Key: fmt.Sprintf("Rollout(%s/%s)", c.Rollout.Namespace, c.Rollout.Name), + Namespace: c.Rollout.Namespace, + ObjectRef: c.Rollout.Spec.Strategy.Canary.TrafficRoutings, + Strategy: currentStep.TrafficRoutingStrategy, + OwnerRef: *metav1.NewControllerRef(c.Rollout, rolloutControllerKind), + RevisionLabelKey: revisionLabelKey, + StableRevision: c.NewStatus.CanaryStatus.StableRevision, + CanaryRevision: c.NewStatus.CanaryStatus.PodTemplateHash, + LastUpdateTime: c.NewStatus.CanaryStatus.LastUpdateTime, + DisableGenerateCanaryService: c.Rollout.Spec.Strategy.Canary.DisableGenerateCanaryService, } } diff --git a/pkg/trafficrouting/manager.go b/pkg/trafficrouting/manager.go index 953b0c19..ad8bc57a 100644 --- a/pkg/trafficrouting/manager.go +++ b/pkg/trafficrouting/manager.go @@ -57,6 +57,8 @@ type TrafficRoutingContext struct { CanaryRevision string // newStatus.canaryStatus.LastUpdateTime LastUpdateTime *metav1.Time + // won't work for Ingress and Gateway + DisableGenerateCanaryService bool } // Manager responsible for adjusting network resources @@ -82,7 +84,7 @@ func (m *Manager) InitializeTrafficRouting(c *TrafficRoutingContext) error { if err := m.Get(context.TODO(), types.NamespacedName{Namespace: c.Namespace, Name: sService}, service); err != nil { return err } - cService := getCanaryServiceName(sService, c.OnlyTrafficRouting) + cService := getCanaryServiceName(sService, c.OnlyTrafficRouting, c.DisableGenerateCanaryService) // new network provider, ingress or gateway trController, err := newNetworkProvider(m.Client, c, sService, cService) if err != nil { @@ -116,7 +118,7 @@ func (m *Manager) DoTrafficRouting(c *TrafficRoutingContext) (bool, error) { return false, err } // canary service name - canaryServiceName := getCanaryServiceName(trafficRouting.Service, c.OnlyTrafficRouting) + canaryServiceName := getCanaryServiceName(trafficRouting.Service, c.OnlyTrafficRouting, c.DisableGenerateCanaryService) canaryService := &corev1.Service{} canaryService.Namespace = stableService.Namespace canaryService.Name = canaryServiceName @@ -131,7 +133,7 @@ func (m *Manager) DoTrafficRouting(c *TrafficRoutingContext) (bool, error) { // end-to-end canary deployment scenario(a -> b -> c), if only b or c is released, //and a is not released in this scenario, then the canary service is not needed. - if !c.OnlyTrafficRouting { + if !c.OnlyTrafficRouting && !c.DisableGenerateCanaryService { if c.StableRevision == "" || c.CanaryRevision == "" { klog.Warningf("%s stableRevision or podTemplateHash can not be empty, and wait a moment", c.Key) return false, nil @@ -206,7 +208,7 @@ func (m *Manager) FinalisingTrafficRouting(c *TrafficRoutingContext, onlyRestore trafficRouting.GracePeriodSeconds = defaultGracePeriodSeconds } - cServiceName := getCanaryServiceName(trafficRouting.Service, c.OnlyTrafficRouting) + cServiceName := getCanaryServiceName(trafficRouting.Service, c.OnlyTrafficRouting, c.DisableGenerateCanaryService) trController, err := newNetworkProvider(m.Client, c, trafficRouting.Service, cServiceName) if err != nil { klog.Errorf("%s newTrafficRoutingController failed: %s", c.Key, err.Error()) @@ -215,6 +217,7 @@ func (m *Manager) FinalisingTrafficRouting(c *TrafficRoutingContext, onlyRestore cService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: c.Namespace, Name: cServiceName}} // if canary svc has been already cleaned up, just return + // even DisableGenerateCanaryService is true, canary svc still exists, because canary service is stable service if err = m.Get(context.TODO(), client.ObjectKeyFromObject(cService), cService); err != nil { if !errors.IsNotFound(err) { klog.Errorf("%s get canary service(%s) failed: %s", c.Key, cServiceName, err.Error()) @@ -259,7 +262,7 @@ func (m *Manager) FinalisingTrafficRouting(c *TrafficRoutingContext, onlyRestore } // end to end deployment, don't remove the canary service; // because canary service is stable service - if !c.OnlyTrafficRouting { + if !c.OnlyTrafficRouting && !c.DisableGenerateCanaryService { // remove canary service err = m.Delete(context.TODO(), cService) if err != nil && !errors.IsNotFound(err) { @@ -281,6 +284,8 @@ func newNetworkProvider(c client.Client, con *TrafficRoutingContext, sService, c StableService: sService, TrafficConf: trafficRouting.CustomNetworkRefs, OwnerRef: con.OwnerRef, + //only set for CustomController, never work for Ingress and Gateway + DisableGenerateCanaryService: con.DisableGenerateCanaryService, }) } if trafficRouting.Ingress != nil { @@ -375,8 +380,8 @@ func (m *Manager) restoreStableService(c *TrafficRoutingContext) (bool, error) { return true, nil } -func getCanaryServiceName(sService string, onlyTrafficRouting bool) string { - if onlyTrafficRouting { +func getCanaryServiceName(sService string, onlyTrafficRouting bool, disableGenerateCanaryService bool) string { + if onlyTrafficRouting || disableGenerateCanaryService { return sService } return fmt.Sprintf("%s-canary", sService) diff --git a/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider.go b/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider.go index 617942dc..5778e4a3 100644 --- a/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider.go +++ b/pkg/trafficrouting/network/customNetworkProvider/custom_network_provider.go @@ -72,8 +72,9 @@ type Config struct { CanaryService string StableService string // network providers need to be created - TrafficConf []v1beta1.ObjectRef - OwnerRef metav1.OwnerReference + TrafficConf []v1beta1.ObjectRef + OwnerRef metav1.OwnerReference + DisableGenerateCanaryService bool } func NewCustomController(client client.Client, conf Config) (network.NetworkProvider, error) {