From bef1dd4931e95138cd6053928a72b4a1638f69c9 Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Mon, 1 Apr 2024 12:18:36 +0800 Subject: [PATCH 1/8] remove always push labels --- pilot/pkg/bootstrap/config_compare.go | 5 +--- pilot/pkg/bootstrap/config_compare_test.go | 31 ++++----------------- pilot/pkg/config/kube/ingress/controller.go | 4 --- pkg/config/constants/constants.go | 3 -- 4 files changed, 6 insertions(+), 37 deletions(-) diff --git a/pilot/pkg/bootstrap/config_compare.go b/pilot/pkg/bootstrap/config_compare.go index f1f4f2b0ab6..53a3fd22320 100644 --- a/pilot/pkg/bootstrap/config_compare.go +++ b/pilot/pkg/bootstrap/config_compare.go @@ -27,11 +27,8 @@ import ( // to be triggered. This is to avoid unnecessary pushes only when labels have changed // for example. func needsPush(prev config.Config, curr config.Config) bool { - if prev.GroupVersionKind != curr.GroupVersionKind { - // This should never happen. - return true - } // If the config is not Istio, let us just push. + // Used for gateway api reconcile. if !strings.HasSuffix(prev.GroupVersionKind.Group, "istio.io") { return true } diff --git a/pilot/pkg/bootstrap/config_compare_test.go b/pilot/pkg/bootstrap/config_compare_test.go index bec2a1cf2f8..a8573d17543 100644 --- a/pilot/pkg/bootstrap/config_compare_test.go +++ b/pilot/pkg/bootstrap/config_compare_test.go @@ -19,7 +19,6 @@ import ( networking "istio.io/api/networking/v1alpha3" "istio.io/istio/pkg/config" - "istio.io/istio/pkg/config/constants" "istio.io/istio/pkg/config/schema/gvk" ) @@ -30,26 +29,6 @@ func TestNeedsPush(t *testing.T) { curr config.Config expected bool }{ - { - name: "different gvk", - prev: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.VirtualService, - Name: "acme2-v1", - Namespace: "not-default", - }, - Spec: &networking.VirtualService{}, - }, - curr: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.DestinationRule, - Name: "acme2-v1", - Namespace: "not-default", - }, - Spec: &networking.VirtualService{}, - }, - expected: true, - }, { name: "same gvk label change", prev: config.Config{ @@ -108,7 +87,7 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", - Labels: map[string]string{constants.AlwaysPushLabel: "true"}, + // Labels: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, expected: true, @@ -120,7 +99,7 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", - Labels: map[string]string{constants.AlwaysPushLabel: "true"}, + // Labels: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, curr: config.Config{ @@ -146,7 +125,7 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", - Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, + // Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, expected: true, @@ -158,7 +137,7 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", - Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, + // Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, curr: config.Config{ @@ -190,7 +169,7 @@ func TestNeedsPush(t *testing.T) { }, Spec: &networking.VirtualService{}, }, - expected: true, + expected: false, }, } diff --git a/pilot/pkg/config/kube/ingress/controller.go b/pilot/pkg/config/kube/ingress/controller.go index ef2c97314b1..58f652a4ab0 100644 --- a/pilot/pkg/config/kube/ingress/controller.go +++ b/pilot/pkg/config/kube/ingress/controller.go @@ -196,15 +196,11 @@ func (c *controller) onEvent(item types.NamespacedName) error { Name: item.Name + "-" + "virtualservice", Namespace: item.Namespace, GroupVersionKind: gvk.VirtualService, - // Set this label so that we do not compare configs and just push. - Labels: map[string]string{constants.AlwaysPushLabel: "true"}, } gatewaymetadata := config.Meta{ Name: item.Name + "-" + "gateway", Namespace: item.Namespace, GroupVersionKind: gvk.Gateway, - // Set this label so that we do not compare configs and just push. - Labels: map[string]string{constants.AlwaysPushLabel: "true"}, } // Trigger updates for Gateway and VirtualService diff --git a/pkg/config/constants/constants.go b/pkg/config/constants/constants.go index 9f1ff44c215..fcd62e258cd 100644 --- a/pkg/config/constants/constants.go +++ b/pkg/config/constants/constants.go @@ -134,9 +134,6 @@ const ( TestVMVersionLabel = "istio.io/test-vm-version" - // Label to skip config comparison. - AlwaysPushLabel = "internal.istio.io/always-push" - // InternalParentNames declares the original resources of an internally-generated config. // This is used by k8s gateway-api. // It is a comma separated list. For example, "HTTPRoute/foo.default,HTTPRoute/bar.default" From 48663676bd2e71ac93bde123433eea5cd4b5d75f Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Mon, 1 Apr 2024 14:41:10 +0800 Subject: [PATCH 2/8] Remove deprecated gateway name support --- .../pkg/config/kube/gateway/deploymentcontroller.go | 12 +----------- pilot/pkg/model/policyattachment.go | 5 ----- pkg/config/constants/constants.go | 2 -- 3 files changed, 1 insertion(+), 18 deletions(-) diff --git a/pilot/pkg/config/kube/gateway/deploymentcontroller.go b/pilot/pkg/config/kube/gateway/deploymentcontroller.go index 93a5df13350..f09619f3f1c 100644 --- a/pilot/pkg/config/kube/gateway/deploymentcontroller.go +++ b/pilot/pkg/config/kube/gateway/deploymentcontroller.go @@ -510,7 +510,7 @@ func (d *DeploymentController) render(templateName string, mi TemplateInput) ([] return nil, fmt.Errorf("no %q template defined", templateName) } - labelToMatch := map[string]string{constants.GatewayNameLabel: mi.Name, constants.DeprecatedGatewayNameLabel: mi.Name} + labelToMatch := map[string]string{constants.GatewayNameLabel: mi.Name} proxyConfig := d.env.GetProxyConfigOrDefault(mi.Namespace, labelToMatch, nil, cfg.MeshConfig) input := derivedInput{ TemplateInput: mi, @@ -618,16 +618,6 @@ func (d *DeploymentController) setGatewayNameLabel(ti *TemplateInput) { log.Debugf("deployment %s/%s not found in store; using to the new gateway name label", ti.DeploymentName, ti.Namespace) return } - - // Base label choice on the deployment's selector - _, exists := dep.(*appsv1.Deployment).Spec.Selector.MatchLabels[constants.DeprecatedGatewayNameLabel] - if !exists { - // The old label doesn't already exist on the deployment; use the new label - return - } - - // The old label exists on the deployment; use the old label - ti.GatewayNameLabel = constants.DeprecatedGatewayNameLabel } type TemplateInput struct { diff --git a/pilot/pkg/model/policyattachment.go b/pilot/pkg/model/policyattachment.go index 832abd2692f..50b9f269f78 100644 --- a/pilot/pkg/model/policyattachment.go +++ b/pilot/pkg/model/policyattachment.go @@ -49,11 +49,6 @@ const ( func KubernetesGatewayNameAndExists(l labels.Instance) (string, bool) { gwName, exists := l[constants.GatewayNameLabel] - if !exists { - // TODO: Remove deprecated gateway name label (1.22 or 1.23) - gwName, exists = l[constants.DeprecatedGatewayNameLabel] - } - return gwName, exists } diff --git a/pkg/config/constants/constants.go b/pkg/config/constants/constants.go index fcd62e258cd..2d5ac1c6844 100644 --- a/pkg/config/constants/constants.go +++ b/pkg/config/constants/constants.go @@ -172,8 +172,6 @@ const ( RemoteGatewayClassName = "istio-remote" WaypointGatewayClassName = "istio-waypoint" - // DeprecatedGatewayNameLabel indicates the gateway managing a particular proxy instances. Only populated for Gateway API gateways - DeprecatedGatewayNameLabel = "istio.io/gateway-name" // GatewayNameLabel indicates the gateway managing a particular proxy instances. Only populated for Gateway API gateways GatewayNameLabel = "gateway.networking.k8s.io/gateway-name" From 92a94803df8da7443067450814d16416f1972707 Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Mon, 1 Apr 2024 14:50:57 +0800 Subject: [PATCH 3/8] update --- pilot/pkg/bootstrap/config_compare_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pilot/pkg/bootstrap/config_compare_test.go b/pilot/pkg/bootstrap/config_compare_test.go index a8573d17543..7439fc55aa7 100644 --- a/pilot/pkg/bootstrap/config_compare_test.go +++ b/pilot/pkg/bootstrap/config_compare_test.go @@ -87,7 +87,6 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", - // Labels: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, expected: true, @@ -99,7 +98,6 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", - // Labels: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, curr: config.Config{ @@ -125,7 +123,6 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", - // Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, expected: true, @@ -137,7 +134,6 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", - // Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, curr: config.Config{ @@ -169,7 +165,7 @@ func TestNeedsPush(t *testing.T) { }, Spec: &networking.VirtualService{}, }, - expected: false, + expected: true, }, } From d5fb49d7e0f1444abc90e6abe25c8b6f5228a28d Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Wed, 10 Apr 2024 17:29:29 +0800 Subject: [PATCH 4/8] add internal.istio.io/route-semantics=ingress annotation and also skip push when ingress spec not change --- pilot/pkg/config/kube/ingress/controller.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pilot/pkg/config/kube/ingress/controller.go b/pilot/pkg/config/kube/ingress/controller.go index 58f652a4ab0..df8703832f2 100644 --- a/pilot/pkg/config/kube/ingress/controller.go +++ b/pilot/pkg/config/kube/ingress/controller.go @@ -19,6 +19,7 @@ package ingress import ( "errors" "fmt" + "reflect" "sort" "sync" @@ -186,25 +187,31 @@ func (c *controller) onEvent(item types.NamespacedName) error { // we should check need process only when event is not delete, // if it is delete event, and previously processed, we need to process too. if event != model.EventDelete { + oldIngress := c.ingresses[config.NamespacedName(ing)] shouldProcess := c.shouldProcessIngressUpdate(ing) if !shouldProcess { return nil } + // only trigger when real changes were found + if oldIngress != nil && reflect.DeepEqual(oldIngress.Spec, ing.Spec) { + return nil + } } vsmetadata := config.Meta{ Name: item.Name + "-" + "virtualservice", Namespace: item.Namespace, GroupVersionKind: gvk.VirtualService, + Annotations: map[string]string{constants.InternalRouteSemantics: constants.RouteSemanticsIngress}, } gatewaymetadata := config.Meta{ Name: item.Name + "-" + "gateway", Namespace: item.Namespace, GroupVersionKind: gvk.Gateway, + Annotations: map[string]string{constants.InternalRouteSemantics: constants.RouteSemanticsIngress}, } // Trigger updates for Gateway and VirtualService - // TODO: we could be smarter here and only trigger when real changes were found for _, f := range c.virtualServiceHandlers { f(config.Config{Meta: vsmetadata}, config.Config{Meta: vsmetadata}, event) } From 49cc16c2ad3c3c95a855a2679b4d5edebbd7b6a8 Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Thu, 11 Apr 2024 15:56:51 +0800 Subject: [PATCH 5/8] restore ingress real change check --- pilot/pkg/config/kube/ingress/controller.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pilot/pkg/config/kube/ingress/controller.go b/pilot/pkg/config/kube/ingress/controller.go index df8703832f2..65d0add24dd 100644 --- a/pilot/pkg/config/kube/ingress/controller.go +++ b/pilot/pkg/config/kube/ingress/controller.go @@ -19,7 +19,6 @@ package ingress import ( "errors" "fmt" - "reflect" "sort" "sync" @@ -187,15 +186,10 @@ func (c *controller) onEvent(item types.NamespacedName) error { // we should check need process only when event is not delete, // if it is delete event, and previously processed, we need to process too. if event != model.EventDelete { - oldIngress := c.ingresses[config.NamespacedName(ing)] shouldProcess := c.shouldProcessIngressUpdate(ing) if !shouldProcess { return nil } - // only trigger when real changes were found - if oldIngress != nil && reflect.DeepEqual(oldIngress.Spec, ing.Spec) { - return nil - } } vsmetadata := config.Meta{ @@ -212,6 +206,7 @@ func (c *controller) onEvent(item types.NamespacedName) error { } // Trigger updates for Gateway and VirtualService + // TODO: we could be smarter here and only trigger when real changes were found for _, f := range c.virtualServiceHandlers { f(config.Config{Meta: vsmetadata}, config.Config{Meta: vsmetadata}, event) } From 6c380d40243c3885d70c546127df5068a41e71d3 Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Fri, 12 Apr 2024 14:55:17 +0800 Subject: [PATCH 6/8] Revert "remove always push labels" This reverts commit bef1dd4931e95138cd6053928a72b4a1638f69c9. --- pilot/pkg/bootstrap/config_compare.go | 5 ++++- pilot/pkg/bootstrap/config_compare_test.go | 25 +++++++++++++++++++++ pilot/pkg/config/kube/ingress/controller.go | 6 +++-- pkg/config/constants/constants.go | 3 +++ 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/pilot/pkg/bootstrap/config_compare.go b/pilot/pkg/bootstrap/config_compare.go index 53a3fd22320..f1f4f2b0ab6 100644 --- a/pilot/pkg/bootstrap/config_compare.go +++ b/pilot/pkg/bootstrap/config_compare.go @@ -27,8 +27,11 @@ import ( // to be triggered. This is to avoid unnecessary pushes only when labels have changed // for example. func needsPush(prev config.Config, curr config.Config) bool { + if prev.GroupVersionKind != curr.GroupVersionKind { + // This should never happen. + return true + } // If the config is not Istio, let us just push. - // Used for gateway api reconcile. if !strings.HasSuffix(prev.GroupVersionKind.Group, "istio.io") { return true } diff --git a/pilot/pkg/bootstrap/config_compare_test.go b/pilot/pkg/bootstrap/config_compare_test.go index 7439fc55aa7..bec2a1cf2f8 100644 --- a/pilot/pkg/bootstrap/config_compare_test.go +++ b/pilot/pkg/bootstrap/config_compare_test.go @@ -19,6 +19,7 @@ import ( networking "istio.io/api/networking/v1alpha3" "istio.io/istio/pkg/config" + "istio.io/istio/pkg/config/constants" "istio.io/istio/pkg/config/schema/gvk" ) @@ -29,6 +30,26 @@ func TestNeedsPush(t *testing.T) { curr config.Config expected bool }{ + { + name: "different gvk", + prev: config.Config{ + Meta: config.Meta{ + GroupVersionKind: gvk.VirtualService, + Name: "acme2-v1", + Namespace: "not-default", + }, + Spec: &networking.VirtualService{}, + }, + curr: config.Config{ + Meta: config.Meta{ + GroupVersionKind: gvk.DestinationRule, + Name: "acme2-v1", + Namespace: "not-default", + }, + Spec: &networking.VirtualService{}, + }, + expected: true, + }, { name: "same gvk label change", prev: config.Config{ @@ -87,6 +108,7 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", + Labels: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, expected: true, @@ -98,6 +120,7 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", + Labels: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, curr: config.Config{ @@ -123,6 +146,7 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", + Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, expected: true, @@ -134,6 +158,7 @@ func TestNeedsPush(t *testing.T) { GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", + Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, curr: config.Config{ diff --git a/pilot/pkg/config/kube/ingress/controller.go b/pilot/pkg/config/kube/ingress/controller.go index 65d0add24dd..ef2c97314b1 100644 --- a/pilot/pkg/config/kube/ingress/controller.go +++ b/pilot/pkg/config/kube/ingress/controller.go @@ -196,13 +196,15 @@ func (c *controller) onEvent(item types.NamespacedName) error { Name: item.Name + "-" + "virtualservice", Namespace: item.Namespace, GroupVersionKind: gvk.VirtualService, - Annotations: map[string]string{constants.InternalRouteSemantics: constants.RouteSemanticsIngress}, + // Set this label so that we do not compare configs and just push. + Labels: map[string]string{constants.AlwaysPushLabel: "true"}, } gatewaymetadata := config.Meta{ Name: item.Name + "-" + "gateway", Namespace: item.Namespace, GroupVersionKind: gvk.Gateway, - Annotations: map[string]string{constants.InternalRouteSemantics: constants.RouteSemanticsIngress}, + // Set this label so that we do not compare configs and just push. + Labels: map[string]string{constants.AlwaysPushLabel: "true"}, } // Trigger updates for Gateway and VirtualService diff --git a/pkg/config/constants/constants.go b/pkg/config/constants/constants.go index 2d5ac1c6844..5d9b50acda2 100644 --- a/pkg/config/constants/constants.go +++ b/pkg/config/constants/constants.go @@ -134,6 +134,9 @@ const ( TestVMVersionLabel = "istio.io/test-vm-version" + // Label to skip config comparison. + AlwaysPushLabel = "internal.istio.io/always-push" + // InternalParentNames declares the original resources of an internally-generated config. // This is used by k8s gateway-api. // It is a comma separated list. For example, "HTTPRoute/foo.default,HTTPRoute/bar.default" From 02fe58632dd2df145f31be44925fc4b578b947bc Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Fri, 12 Apr 2024 15:08:10 +0800 Subject: [PATCH 7/8] update --- pilot/pkg/bootstrap/config_compare.go | 25 ++----- pilot/pkg/bootstrap/config_compare_test.go | 84 ++-------------------- 2 files changed, 9 insertions(+), 100 deletions(-) diff --git a/pilot/pkg/bootstrap/config_compare.go b/pilot/pkg/bootstrap/config_compare.go index f1f4f2b0ab6..59ad090ea9a 100644 --- a/pilot/pkg/bootstrap/config_compare.go +++ b/pilot/pkg/bootstrap/config_compare.go @@ -21,6 +21,7 @@ import ( "google.golang.org/protobuf/proto" "istio.io/istio/pkg/config" + "istio.io/istio/pkg/config/constants" ) // needsPush checks whether the passed in config has same spec and hence push needs @@ -35,27 +36,11 @@ func needsPush(prev config.Config, curr config.Config) bool { if !strings.HasSuffix(prev.GroupVersionKind.Group, "istio.io") { return true } - // If current/previous metadata has "*istio.io" label/annotation, just push - for label := range curr.Meta.Labels { - if strings.Contains(label, "istio.io") { - return true - } - } - for annotation := range curr.Meta.Annotations { - if strings.Contains(annotation, "istio.io") { - return true - } - } - for label := range prev.Meta.Labels { - if strings.Contains(label, "istio.io") { - return true - } - } - for annotation := range prev.Meta.Annotations { - if strings.Contains(annotation, "istio.io") { - return true - } + + if _, ok := curr.Labels[constants.AlwaysPushLabel]; ok { + return true } + prevspecProto, okProtoP := prev.Spec.(proto.Message) currspecProto, okProtoC := curr.Spec.(proto.Message) if okProtoP && okProtoC { diff --git a/pilot/pkg/bootstrap/config_compare_test.go b/pilot/pkg/bootstrap/config_compare_test.go index bec2a1cf2f8..4a1481f61ad 100644 --- a/pilot/pkg/bootstrap/config_compare_test.go +++ b/pilot/pkg/bootstrap/config_compare_test.go @@ -30,26 +30,6 @@ func TestNeedsPush(t *testing.T) { curr config.Config expected bool }{ - { - name: "different gvk", - prev: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.VirtualService, - Name: "acme2-v1", - Namespace: "not-default", - }, - Spec: &networking.VirtualService{}, - }, - curr: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.DestinationRule, - Name: "acme2-v1", - Namespace: "not-default", - }, - Spec: &networking.VirtualService{}, - }, - expected: true, - }, { name: "same gvk label change", prev: config.Config{ @@ -95,29 +75,10 @@ func TestNeedsPush(t *testing.T) { expected: true, }, { - name: "current config with istio.io label", - prev: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.Ingress, - Name: "acme2-v1", - Namespace: "not-default", - }, - }, - curr: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.Ingress, - Name: "acme2-v1", - Namespace: "not-default", - Labels: map[string]string{constants.AlwaysPushLabel: "true"}, - }, - }, - expected: true, - }, - { - name: "previous config with istio.io label", + name: "with AlwaysPushLabel label", prev: config.Config{ Meta: config.Meta{ - GroupVersionKind: gvk.Ingress, + GroupVersionKind: gvk.VirtualService, Name: "acme2-v1", Namespace: "not-default", Labels: map[string]string{constants.AlwaysPushLabel: "true"}, @@ -125,47 +86,10 @@ func TestNeedsPush(t *testing.T) { }, curr: config.Config{ Meta: config.Meta{ - GroupVersionKind: gvk.Ingress, - Name: "acme2-v1", - Namespace: "not-default", - }, - }, - expected: true, - }, - { - name: "current config with istio.io annotation", - prev: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.Ingress, - Name: "acme2-v1", - Namespace: "not-default", - }, - }, - curr: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.Ingress, - Name: "acme2-v1", - Namespace: "not-default", - Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, - }, - }, - expected: true, - }, - { - name: "previous config with istio.io annotation", - prev: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.Ingress, - Name: "acme2-v1", - Namespace: "not-default", - Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, - }, - }, - curr: config.Config{ - Meta: config.Meta{ - GroupVersionKind: gvk.Ingress, + GroupVersionKind: gvk.VirtualService, Name: "acme2-v1", Namespace: "not-default", + Labels: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, expected: true, From 0807711fc66cf1d1e80bc085fd1f35159899afaa Mon Sep 17 00:00:00 2001 From: Zhonghu Xu Date: Tue, 16 Apr 2024 11:03:08 +0800 Subject: [PATCH 8/8] revert config compare --- pilot/pkg/bootstrap/config_compare.go | 28 +++++++--- pilot/pkg/bootstrap/config_compare_test.go | 64 ++++++++++++++++++++-- 2 files changed, 81 insertions(+), 11 deletions(-) diff --git a/pilot/pkg/bootstrap/config_compare.go b/pilot/pkg/bootstrap/config_compare.go index 59ad090ea9a..2508d9c1696 100644 --- a/pilot/pkg/bootstrap/config_compare.go +++ b/pilot/pkg/bootstrap/config_compare.go @@ -21,24 +21,38 @@ import ( "google.golang.org/protobuf/proto" "istio.io/istio/pkg/config" - "istio.io/istio/pkg/config/constants" ) // needsPush checks whether the passed in config has same spec and hence push needs // to be triggered. This is to avoid unnecessary pushes only when labels have changed // for example. func needsPush(prev config.Config, curr config.Config) bool { - if prev.GroupVersionKind != curr.GroupVersionKind { - // This should never happen. - return true - } // If the config is not Istio, let us just push. if !strings.HasSuffix(prev.GroupVersionKind.Group, "istio.io") { return true } - if _, ok := curr.Labels[constants.AlwaysPushLabel]; ok { - return true + // If current/previous metadata has "*istio.io" label/annotation, just push + // Currently applies to label "internal.istio.io/always-push", annotation "istio.io/dry-run" + for label := range curr.Meta.Labels { + if strings.Contains(label, "istio.io") { + return true + } + } + for annotation := range curr.Meta.Annotations { + if strings.Contains(annotation, "istio.io") { + return true + } + } + for label := range prev.Meta.Labels { + if strings.Contains(label, "istio.io") { + return true + } + } + for annotation := range prev.Meta.Annotations { + if strings.Contains(annotation, "istio.io") { + return true + } } prevspecProto, okProtoP := prev.Spec.(proto.Message) diff --git a/pilot/pkg/bootstrap/config_compare_test.go b/pilot/pkg/bootstrap/config_compare_test.go index 4a1481f61ad..73539cb2dd8 100644 --- a/pilot/pkg/bootstrap/config_compare_test.go +++ b/pilot/pkg/bootstrap/config_compare_test.go @@ -75,23 +75,79 @@ func TestNeedsPush(t *testing.T) { expected: true, }, { - name: "with AlwaysPushLabel label", + name: "current config with istio.io label", prev: config.Config{ Meta: config.Meta{ - GroupVersionKind: gvk.VirtualService, + GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", - Labels: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, curr: config.Config{ Meta: config.Meta{ - GroupVersionKind: gvk.VirtualService, + GroupVersionKind: gvk.Ingress, + Name: "acme2-v1", + Namespace: "not-default", + Labels: map[string]string{constants.AlwaysPushLabel: "true"}, + }, + }, + expected: true, + }, + { + name: "previous config with istio.io label", + prev: config.Config{ + Meta: config.Meta{ + GroupVersionKind: gvk.Ingress, Name: "acme2-v1", Namespace: "not-default", Labels: map[string]string{constants.AlwaysPushLabel: "true"}, }, }, + curr: config.Config{ + Meta: config.Meta{ + GroupVersionKind: gvk.Ingress, + Name: "acme2-v1", + Namespace: "not-default", + }, + }, + expected: true, + }, + { + name: "current config with istio.io annotation", + prev: config.Config{ + Meta: config.Meta{ + GroupVersionKind: gvk.Ingress, + Name: "acme2-v1", + Namespace: "not-default", + }, + }, + curr: config.Config{ + Meta: config.Meta{ + GroupVersionKind: gvk.Ingress, + Name: "acme2-v1", + Namespace: "not-default", + Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, + }, + }, + expected: true, + }, + { + name: "previous config with istio.io annotation", + prev: config.Config{ + Meta: config.Meta{ + GroupVersionKind: gvk.Ingress, + Name: "acme2-v1", + Namespace: "not-default", + Annotations: map[string]string{constants.AlwaysPushLabel: "true"}, + }, + }, + curr: config.Config{ + Meta: config.Meta{ + GroupVersionKind: gvk.Ingress, + Name: "acme2-v1", + Namespace: "not-default", + }, + }, expected: true, }, {