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 <yunbo10124scut@gmail.com>

* add DisableGenerateCanaryService for CanaryStrategy

amend1: update crd yaml

amend2: add DisableGenerateCanaryService for v1alpha1

Signed-off-by: yunbo <yunbo10124scut@gmail.com>

---------

Signed-off-by: yunbo <yunbo10124scut@gmail.com>
Co-authored-by: yunbo <yunbo10124scut@gmail.com>

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

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 {

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 {

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

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),
+		Key:                          fmt.Sprintf("Rollout(%s/%s)", c.Rollout.Namespace, c.Rollout.Name),
-		Namespace:        c.Rollout.Namespace,
+		Namespace:                    c.Rollout.Namespace,
-		ObjectRef:        c.Rollout.Spec.Strategy.Canary.TrafficRoutings,
+		ObjectRef:                    c.Rollout.Spec.Strategy.Canary.TrafficRoutings,
-		Strategy:         currentStep.TrafficRoutingStrategy,
+		Strategy:                     currentStep.TrafficRoutingStrategy,
-		OwnerRef:         *metav1.NewControllerRef(c.Rollout, rolloutControllerKind),
+		OwnerRef:                     *metav1.NewControllerRef(c.Rollout, rolloutControllerKind),
-		RevisionLabelKey: revisionLabelKey,
+		RevisionLabelKey:             revisionLabelKey,
-		StableRevision:   c.NewStatus.CanaryStatus.StableRevision,
+		StableRevision:               c.NewStatus.CanaryStatus.StableRevision,
-		CanaryRevision:   c.NewStatus.CanaryStatus.PodTemplateHash,
+		CanaryRevision:               c.NewStatus.CanaryStatus.PodTemplateHash,
-		LastUpdateTime:   c.NewStatus.CanaryStatus.LastUpdateTime,
+		LastUpdateTime:               c.NewStatus.CanaryStatus.LastUpdateTime,
+		DisableGenerateCanaryService: c.Rollout.Spec.Strategy.Canary.DisableGenerateCanaryService,
 	}
 }

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 {
+func getCanaryServiceName(sService string, onlyTrafficRouting bool, disableGenerateCanaryService bool) string {
-	if onlyTrafficRouting {
+	if onlyTrafficRouting || disableGenerateCanaryService {
 		return sService
 	}
 	return fmt.Sprintf("%s-canary", sService)

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
+	TrafficConf                  []v1beta1.ObjectRef
-	OwnerRef    metav1.OwnerReference
+	OwnerRef                     metav1.OwnerReference
+	DisableGenerateCanaryService bool
 }
 
 func NewCustomController(client client.Client, conf Config) (network.NetworkProvider, error) {