Cannot delete gateways and hosts from VirtualService generated by Canary when enable Canary delegation

[EigoOda]

Describe the bug

Delegation was enabled for applications that have Flagger deployed.
The delegation is now enabled, but I think the VirtualService created by Canary is displayed incorrectly.

When enabling the delegation for Canary, which has already been created, I also deleted the gateways and hosts.
The gateways and hosts of Canary were deleted, but the gateways and hosts of the VirtualService created by Canary remain on the surface.

Flagger log(only this)

{"level":"info","ts":"2023-07-21T06:34:22.430Z","caller":"controller/controller.go:307","msg":"Synced <ns>/<canary>"}

To Reproduce

1. Deploy canary

apiVersion: flagger.app/v1beta1
kind: Canary
metadata:
  name: app1
  namespace: app1
spec:
  targetRef:
    apiVersion: apps/v1
    kind: Deployment
    name: app1
  autoscalerRef:
    apiVersion: autoscaling/v2
    kind: HorizontalPodAutoscaler
    name: app1
  service:
    name: app1
    gateways:
      - app1
    hosts:
    - app1.com
    port: 80
    targetPort: 8080
    retries:
      attempts: 0
    timeout: 3s
  skipAnalysis: false
  analysis: 
    interval: 1m
    threshold: 3
    maxWeight: 20
    stepWeight: 1
...

2. Enable delegation and Deploy VirtualService

apiVersion: flagger.app/v1beta1
kind: Canary
metadata:
  name: app1
  namespace: app1
spec:
  targetRef:
    apiVersion: apps/v1
    kind: Deployment
    name: app1
  autoscalerRef:
    apiVersion: autoscaling/v2
    kind: HorizontalPodAutoscaler
    name: app1
  service:
    name: app1
    delegation: true
    port: 80
    targetPort: 8080
    retries:
      attempts: 0
    timeout: 3s
...
---
apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: app1-delegate
  namespace: app1
spec:
  gateways:
    - app1
  hosts:
    - app1.com
  http:
    - name: dev
      match:
        - headers:
            env:
              exact: dev
      retries:
        attempts: 0
      delegate:
        name: app1-dev
        namespace: app1-dev
    - name: test
      match:
        - headers:
            env:
              exact: test
      retries:
        attempts: 0
      delegate:
        name: app1-test
        namespace: app1-test
    - name: default
      retries:
        attempts: 0
      delegate:
        name: app1
        namespace: app1

3. Didn't delete GATEWAYS and HOSTS from app1 VirtualService

$ k get virtualservice -n app1
NAME            GATEWAYS    HOSTS               AGE
app1            ["app1"]   ["app1.com","app1"]  7d3h
app1-delegate   ["app1"]   ["app1.com"]         16m

When I check annotations, they appear to have been deleted.

$ k get vs -n app1 app1 -oyaml
apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  annotations:
    flagger.kubernetes.io/original-configuration: '{"hosts":[],"http":[{"route":[{"destination":{"host":"app1-primary"},"weight":100},{"destination":{"host":"app1-canary"},"weight":0}],"timeout":"3s"}]}'
    kustomize.toolkit.fluxcd.io/reconcile: disabled
  creationTimestamp: "2023-07-14T02:55:42Z"
  generation: 18
  name: app1
  namespace: app1
  ...
spec:
  gateways:
  - app1
  hosts:
  - app1.com
  - app1
  http:
  - retries: {}
    route:
    - destination:
        host: app1-primary
      weight: 100
    - destination:
        host: app1-canary
      weight: 0
    timeout: 3s

4. Gateways and Hosts was delete from Canary

$ k get canary -n app1 app1 -oyaml
apiVersion: flagger.app/v1beta1
kind: Canary
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"flagger.app/v1beta1","kind":"Canary","metadata":{"annotations":{},"labels":{"app":"***","env":"***"},"name":"***","namespace":"***"},"spec":{"analysis":{"alerts":[{"name":"on-call Slack","providerRef":{"name":"***","namespace":"istio-system"},"severity":"info"}],"interval":"1m","maxWeight":20,"metrics":[{"interval":"1m","name":"error-count","templateRef":{"name":"***"},"thresholdRange":{"max":10}},{"interval":"1m","name":"***","templateRef":{"name":"***"},"thresholdRange":{"max":1}}],"stepWeight":10,"threshold":3,"webhooks":[{"metadata":{"cmd":"***","type":"bash"},"name":"***","timeout":"30s","type":"pre-rollout","url":"http://flagger-loadtester.istio-system/"}]},"autoscalerRef":{"apiVersion":"autoscaling/v2","kind":"HorizontalPodAutoscaler","name":"***"},"progressDeadlineSeconds":1800,"service":{"delegation":true,"name":"***","port":80,"retries":{"attempts":0},"targetPort":8080,"timeout":"3s"},"skipAnalysis":false,"targetRef":{"apiVersion":"apps/v1","kind":"Deployment","name":"***"}}}
  creationTimestamp: "2023-07-13T08:44:37Z"
  ...
spec:
  ...
  autoscalerRef:
    apiVersion: autoscaling/v2
    kind: HorizontalPodAutoscaler
    name: app1
  service:
    delegation: true
    name: app1
    port: 80
    retries:
      attempts: 0
    targetPort: 8080
    timeout: 3s
  skipAnalysis: false
  targetRef:
    apiVersion: apps/v1
    kind: Deployment
    name: app1

Workaround?

I found that deleting Gateways and hosts from the VirtualService created by Canary.
Delete spec.service.retries from canary, Gateways and hosts are deleted from the VirtualService.

Expected behavior

Delete Gateways and Hosts from the VirtualService created by Canary.

Additional context

• Flagger version: 1.22.1
• Kubernetes version: 1.23.17
• Service Mesh provider: Istio
• Ingress provider: Istio

[danielkimuipath]

I had the same issue when you have an existing virtual service that contains gateway and hostname information, applying a new virtual service change for delegation by flagger; it did not clean up gateway and hostname value.

[S-mishina]

if delegation: true is set while a VirtualService has already been created by Flagger, there are no changes to the newSpec. Therefore, I suspect that the hosts and gateway are not removed.

flagger/pkg/router/istio.go

Lines 296 to 300 in 9a0c6e7

	if canary.Spec.Service.Delegation {
	// delegate VirtualService requires the hosts and gateway empty.
	virtualService.Spec.Gateways = []string{}
	virtualService.Spec.Hosts = []string{}
	}

flagger/pkg/router/istio.go

Line 333 in 9a0c6e7

if specDiff != "" || labelsDiff != "" || annotationsDiff != "" {

[MEMO] Code of the relevant part

flagger/pkg/router/istio.go

Lines 325 to 364 in 9a0c6e7

	if virtualService != nil {
	specDiff := cmp.Diff(
	newSpec,
	virtualService.Spec,
	ignoreCmpOptions...,
	)
	labelsDiff := cmp.Diff(newMetadata.Labels, virtualService.Labels, cmpopts.EquateEmpty())
	annotationsDiff := cmp.Diff(newMetadata.Annotations, virtualService.Annotations, cmpopts.EquateEmpty())
	if specDiff != "" || labelsDiff != "" || annotationsDiff != "" {
	vtClone := virtualService.DeepCopy()
	vtClone.Spec = newSpec
	vtClone.ObjectMeta.Annotations = newMetadata.Annotations
	vtClone.ObjectMeta.Labels = newMetadata.Labels
	//If annotation kubectl.kubernetes.io/last-applied-configuration is present no need to duplicate
	//serialization. If not present store the serialized object in annotation
	//flagger.kubernetes.app/original-configuration
	if _, ok := vtClone.Annotations[kubectlAnnotation]; !ok && specDiff != "" {
	b, err := json.Marshal(virtualService.Spec)
	if err != nil {
	ir.logger.Warnf("Unable to marshal VS %s for orig-configuration annotation", virtualService.Name)
	}
	if vtClone.ObjectMeta.Annotations == nil {
	vtClone.ObjectMeta.Annotations = make(map[string]string)
	} else {
	vtClone.ObjectMeta.Annotations = filterMetadata(vtClone.ObjectMeta.Annotations)
	}
	vtClone.ObjectMeta.Annotations[configAnnotation] = string(b)
	}
	_, err = ir.istioClient.NetworkingV1beta1().VirtualServices(canary.Namespace).Update(context.TODO(), vtClone, metav1.UpdateOptions{})
	if err != nil {
	return fmt.Errorf("VirtualService %s.%s update error: %w", apexName, canary.Namespace, err)
	}
	ir.logger.With("canary", fmt.Sprintf("%s.%s", canary.Name, canary.Namespace)).
	Infof("VirtualService %s.%s updated", virtualService.GetName(), canary.Namespace)
	}
	}