From 829dfd706f61ab3a4b47e6c550bb2a8cc7d3a072 Mon Sep 17 00:00:00 2001 From: zirain Date: Thu, 8 May 2025 12:00:26 +0800 Subject: [PATCH 1/4] fix: proxy creation/deletion error handling in GatewayNamespace mode Signed-off-by: zirain --- internal/infrastructure/kubernetes/infra.go | 37 +++++++++-------- .../kubernetes/infra_resource.go | 28 ++++++------- .../kubernetes/proxy/resource_provider.go | 27 +++++++------ .../proxy/resource_provider_test.go | 40 ++++++++++++++++--- .../deployments/gateway-namespace-mode.yaml | 2 +- .../testdata/serviceaccount/default.yaml | 3 +- .../gateway-namespace-mode.yaml | 12 ++++++ .../serviceaccount/with-annotations.yaml | 3 +- .../infrastructure/kubernetes/proxy_infra.go | 21 +++++----- .../kubernetes/ratelimit/resource_provider.go | 22 +++++----- 10 files changed, 123 insertions(+), 72 deletions(-) create mode 100644 internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/gateway-namespace-mode.yaml diff --git a/internal/infrastructure/kubernetes/infra.go b/internal/infrastructure/kubernetes/infra.go index 4ecbdc5b67..9c460775f2 100644 --- a/internal/infrastructure/kubernetes/infra.go +++ b/internal/infrastructure/kubernetes/infra.go @@ -31,6 +31,7 @@ var _ ResourceRender = &ratelimit.ResourceRender{} // based on Infra IR resources. type ResourceRender interface { Name() string + Namespace() string LabelSelector() labels.Selector ServiceAccount() (*corev1.ServiceAccount, error) Service() (*corev1.Service, error) @@ -61,12 +62,10 @@ type Infra struct { // NewInfra returns a new Infra. func NewInfra(cli client.Client, cfg *config.Server) *Infra { - var ns string - if !cfg.EnvoyGateway.GatewayNamespaceMode() { - ns = cfg.ControllerNamespace - } return &Infra{ - Namespace: ns, + // Always set infra namespace to cfg.ControllerNamespace, + // Otherwise RateLimit resource provider will failed to create/delete. + Namespace: cfg.ControllerNamespace, DNSDomain: cfg.DNSDomain, EnvoyGateway: cfg.EnvoyGateway, Client: New(cli), @@ -81,31 +80,31 @@ func (i *Infra) Close() error { return nil } // provided ResourceRender, if it doesn't exist and updates it if it does. func (i *Infra) createOrUpdate(ctx context.Context, r ResourceRender) error { if err := i.createOrUpdateServiceAccount(ctx, r); err != nil { - return fmt.Errorf("failed to create or update serviceaccount %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to create or update serviceaccount %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.createOrUpdateConfigMap(ctx, r); err != nil { - return fmt.Errorf("failed to create or update configmap %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to create or update configmap %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.createOrUpdateDeployment(ctx, r); err != nil { - return fmt.Errorf("failed to create or update deployment %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to create or update deployment %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.createOrUpdateDaemonSet(ctx, r); err != nil { - return fmt.Errorf("failed to create or update daemonset %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to create or update daemonset %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.createOrUpdateService(ctx, r); err != nil { - return fmt.Errorf("failed to create or update service %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to create or update service %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.createOrUpdateHPA(ctx, r); err != nil { - return fmt.Errorf("failed to create or update hpa %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to create or update hpa %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.createOrUpdatePodDisruptionBudget(ctx, r); err != nil { - return fmt.Errorf("failed to create or update pdb %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to create or update pdb %s/%s: %w", r.Namespace(), r.Name(), err) } return nil @@ -114,31 +113,31 @@ func (i *Infra) createOrUpdate(ctx context.Context, r ResourceRender) error { // delete deletes the ServiceAccount/ConfigMap/Deployment/Service in the kube api server, if it exists. func (i *Infra) delete(ctx context.Context, r ResourceRender) error { if err := i.deleteServiceAccount(ctx, r); err != nil { - return fmt.Errorf("failed to delete serviceaccount %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to delete serviceaccount %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.deleteConfigMap(ctx, r); err != nil { - return fmt.Errorf("failed to delete configmap %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to delete configmap %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.deleteDeployment(ctx, r); err != nil { - return fmt.Errorf("failed to delete deployment %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to delete deployment %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.deleteDaemonSet(ctx, r); err != nil { - return fmt.Errorf("failed to delete daemonset %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to delete daemonset %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.deleteService(ctx, r); err != nil { - return fmt.Errorf("failed to delete service %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to delete service %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.deleteHPA(ctx, r); err != nil { - return fmt.Errorf("failed to delete hpa %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to delete hpa %s/%s: %w", r.Namespace(), r.Name(), err) } if err := i.deletePDB(ctx, r); err != nil { - return fmt.Errorf("failed to delete pdb %s/%s: %w", i.Namespace, r.Name(), err) + return fmt.Errorf("failed to delete pdb %s/%s: %w", r.Namespace(), r.Name(), err) } return nil diff --git a/internal/infrastructure/kubernetes/infra_resource.go b/internal/infrastructure/kubernetes/infra_resource.go index c37bd81f5e..018e5e00fa 100644 --- a/internal/infrastructure/kubernetes/infra_resource.go +++ b/internal/infrastructure/kubernetes/infra_resource.go @@ -33,7 +33,7 @@ func (i *Infra) createOrUpdateServiceAccount(ctx context.Context, r ResourceRend labels = []metrics.LabelValue{ kindLabel.Value("ServiceAccount"), nameLabel.Value(r.Name()), - namespaceLabel.Value(i.Namespace), + namespaceLabel.Value(r.Namespace()), } ) @@ -68,7 +68,7 @@ func (i *Infra) createOrUpdateConfigMap(ctx context.Context, r ResourceRender) ( labels = []metrics.LabelValue{ kindLabel.Value("ConfigMap"), nameLabel.Value(r.Name()), - namespaceLabel.Value(i.Namespace), + namespaceLabel.Value(r.Namespace()), } ) @@ -102,7 +102,7 @@ func (i *Infra) createOrUpdateDeployment(ctx context.Context, r ResourceRender) labels = []metrics.LabelValue{ kindLabel.Value("Deployment"), nameLabel.Value(r.Name()), - namespaceLabel.Value(i.Namespace), + namespaceLabel.Value(r.Namespace()), } ) @@ -189,7 +189,7 @@ func (i *Infra) createOrUpdateDaemonSet(ctx context.Context, r ResourceRender) ( labels = []metrics.LabelValue{ kindLabel.Value("DaemonSet"), nameLabel.Value(r.Name()), - namespaceLabel.Value(i.Namespace), + namespaceLabel.Value(r.Namespace()), } ) @@ -282,7 +282,7 @@ func (i *Infra) createOrUpdatePodDisruptionBudget(ctx context.Context, r Resourc labels = []metrics.LabelValue{ kindLabel.Value("PDB"), nameLabel.Value(r.Name()), - namespaceLabel.Value(i.Namespace), + namespaceLabel.Value(r.Namespace()), } ) @@ -319,7 +319,7 @@ func (i *Infra) createOrUpdateHPA(ctx context.Context, r ResourceRender) (err er labels = []metrics.LabelValue{ kindLabel.Value("HPA"), nameLabel.Value(r.Name()), - namespaceLabel.Value(i.Namespace), + namespaceLabel.Value(r.Namespace()), } ) @@ -355,7 +355,7 @@ func (i *Infra) createOrUpdateService(ctx context.Context, r ResourceRender) (er labels = []metrics.LabelValue{ kindLabel.Value("Service"), nameLabel.Value(r.Name()), - namespaceLabel.Value(i.Namespace), + namespaceLabel.Value(r.Namespace()), } ) @@ -390,7 +390,7 @@ func (i *Infra) createOrUpdateService(ctx context.Context, r ResourceRender) (er // deleteServiceAccount deletes the ServiceAccount in the kube api server, if it exists. func (i *Infra) deleteServiceAccount(ctx context.Context, r ResourceRender) (err error) { var ( - name, ns = r.Name(), i.Namespace + name, ns = r.Name(), r.Namespace() sa = &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, @@ -425,7 +425,7 @@ func (i *Infra) deleteServiceAccount(ctx context.Context, r ResourceRender) (err // deleteDeployment deletes the Envoy Deployment in the kube api server, if it exists. func (i *Infra) deleteDeployment(ctx context.Context, r ResourceRender) (err error) { var ( - name, ns = r.Name(), i.Namespace + name, ns = r.Name(), r.Namespace() deployment = &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, @@ -460,7 +460,7 @@ func (i *Infra) deleteDeployment(ctx context.Context, r ResourceRender) (err err // deleteDaemonSet deletes the Envoy DaemonSet in the kube api server, if it exists. func (i *Infra) deleteDaemonSet(ctx context.Context, r ResourceRender) (err error) { var ( - name, ns = r.Name(), i.Namespace + name, ns = r.Name(), r.Namespace() daemonSet = &appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, @@ -495,7 +495,7 @@ func (i *Infra) deleteDaemonSet(ctx context.Context, r ResourceRender) (err erro // deleteConfigMap deletes the ConfigMap in the kube api server, if it exists. func (i *Infra) deleteConfigMap(ctx context.Context, r ResourceRender) (err error) { var ( - name, ns = r.Name(), i.Namespace + name, ns = r.Name(), r.Namespace() cm = &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, @@ -530,7 +530,7 @@ func (i *Infra) deleteConfigMap(ctx context.Context, r ResourceRender) (err erro // deleteService deletes the Service in the kube api server, if it exists. func (i *Infra) deleteService(ctx context.Context, r ResourceRender) (err error) { var ( - name, ns = r.Name(), i.Namespace + name, ns = r.Name(), r.Namespace() svc = &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, @@ -565,7 +565,7 @@ func (i *Infra) deleteService(ctx context.Context, r ResourceRender) (err error) // deleteHpa deletes the Horizontal Pod Autoscaler associated to its renderer, if it exists. func (i *Infra) deleteHPA(ctx context.Context, r ResourceRender) (err error) { var ( - name, ns = r.Name(), i.Namespace + name, ns = r.Name(), r.Namespace() hpa = &autoscalingv2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, @@ -600,7 +600,7 @@ func (i *Infra) deleteHPA(ctx context.Context, r ResourceRender) (err error) { // deletePDB deletes the PodDistribution budget associated to its renderer, if it exists. func (i *Infra) deletePDB(ctx context.Context, r ResourceRender) (err error) { var ( - name, ns = r.Name(), i.Namespace + name, ns = r.Name(), r.Namespace() pdb = &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider.go b/internal/infrastructure/kubernetes/proxy/resource_provider.go index ca4543d072..2fba10056f 100644 --- a/internal/infrastructure/kubernetes/proxy/resource_provider.go +++ b/internal/infrastructure/kubernetes/proxy/resource_provider.go @@ -46,8 +46,7 @@ const ( type ResourceRender struct { infra *ir.ProxyInfra - // Namespace is the Namespace used for managed infra. - Namespace string + ns string // DNSDomain is the dns domain used by k8s services. Defaults to "cluster.local". DNSDomain string @@ -59,7 +58,7 @@ type ResourceRender struct { func NewResourceRender(ns, dnsDomain string, infra *ir.ProxyInfra, gateway *egv1a1.EnvoyGateway) *ResourceRender { return &ResourceRender{ - Namespace: ns, + ns: ns, DNSDomain: dnsDomain, infra: infra, ShutdownManager: gateway.GetEnvoyGatewayProvider().GetEnvoyGatewayKubeProvider().ShutdownManager, @@ -71,6 +70,10 @@ func (r *ResourceRender) Name() string { return ExpectedResourceHashedName(r.infra.Name) } +func (r *ResourceRender) Namespace() string { + return r.ns +} + func (r *ResourceRender) LabelSelector() labels.Selector { return labels.SelectorFromSet(r.stableSelector().MatchLabels) } @@ -89,7 +92,7 @@ func (r *ResourceRender) ServiceAccount() (*corev1.ServiceAccount, error) { APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Name: r.Name(), Labels: labels, Annotations: r.infra.GetProxyMetadata().Annotations, @@ -196,7 +199,7 @@ func (r *ResourceRender) Service() (*corev1.Service, error) { Kind: "Service", }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Labels: svcLabels, Annotations: annotations, }, @@ -241,7 +244,7 @@ func (r *ResourceRender) ConfigMap(cert string) (*corev1.ConfigMap, error) { APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Name: r.Name(), Labels: labels, Annotations: r.infra.GetProxyMetadata().Annotations, @@ -280,7 +283,7 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) { } // Get expected bootstrap configurations rendered ProxyContainers - containers, err := expectedProxyContainers(r.infra, deploymentConfig.Container, proxyConfig.Spec.Shutdown, r.ShutdownManager, r.Namespace, r.DNSDomain, r.GatewayNamespaceMode) + containers, err := expectedProxyContainers(r.infra, deploymentConfig.Container, proxyConfig.Spec.Shutdown, r.ShutdownManager, r.Namespace(), r.DNSDomain, r.GatewayNamespaceMode) if err != nil { return nil, err } @@ -300,7 +303,7 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) { APIVersion: "apps/v1", }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Labels: dpLabels, Annotations: dpAnnotations, }, @@ -369,7 +372,7 @@ func (r *ResourceRender) DaemonSet() (*appsv1.DaemonSet, error) { } // Get expected bootstrap configurations rendered ProxyContainers - containers, err := expectedProxyContainers(r.infra, daemonSetConfig.Container, proxyConfig.Spec.Shutdown, r.ShutdownManager, r.Namespace, r.DNSDomain, r.GatewayNamespaceMode) + containers, err := expectedProxyContainers(r.infra, daemonSetConfig.Container, proxyConfig.Spec.Shutdown, r.ShutdownManager, r.Namespace(), r.DNSDomain, r.GatewayNamespaceMode) if err != nil { return nil, err } @@ -389,7 +392,7 @@ func (r *ResourceRender) DaemonSet() (*appsv1.DaemonSet, error) { APIVersion: "apps/v1", }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Labels: dsLabels, Annotations: dsAnnotations, }, @@ -458,7 +461,7 @@ func (r *ResourceRender) PodDisruptionBudget() (*policyv1.PodDisruptionBudget, e podDisruptionBudget := &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: r.Name(), - Namespace: r.Namespace, + Namespace: r.Namespace(), }, TypeMeta: metav1.TypeMeta{ APIVersion: "policy/v1", @@ -492,7 +495,7 @@ func (r *ResourceRender) HorizontalPodAutoscaler() (*autoscalingv2.HorizontalPod Kind: "HorizontalPodAutoscaler", }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Name: r.Name(), Annotations: r.infra.GetProxyMetadata().Annotations, Labels: r.infra.GetProxyMetadata().Labels, diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go index 0651a433af..f05f9d758a 100644 --- a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go +++ b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go @@ -567,7 +567,7 @@ func TestDeployment(t *testing.T) { }, { caseName: "gateway-namespace-mode", - infra: newTestInfraWithNamespace("default"), + infra: newTestInfraWithNamespace("ns1"), gatewayNamespaceMode: true, }, } @@ -1291,27 +1291,57 @@ func TestServiceAccount(t *testing.T) { cfg, err := config.New(os.Stdout) require.NoError(t, err) cases := []struct { - name string - infra *ir.Infra + name string + infra *ir.Infra + gatewayNamespaceMode bool }{ { name: "default", infra: newTestInfra(), - }, { + }, + { name: "with-annotations", infra: newTestInfraWithAnnotations(map[string]string{ "anno1": "value1", "anno2": "value2", }), }, + { + name: "gateway-namespace-mode", + infra: newTestInfraWithNamespace("ns1"), + gatewayNamespaceMode: true, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - r := NewResourceRender(cfg.ControllerNamespace, cfg.DNSDomain, tc.infra.GetProxyInfra(), cfg.EnvoyGateway) + ns := cfg.ControllerNamespace + if tc.gatewayNamespaceMode { + deployType := egv1a1.KubernetesDeployModeType(egv1a1.KubernetesDeployModeTypeGatewayNamespace) + cfg.EnvoyGateway.Provider = &egv1a1.EnvoyGatewayProvider{ + Type: egv1a1.ProviderTypeKubernetes, + Kubernetes: &egv1a1.EnvoyGatewayKubernetesProvider{ + Deploy: &egv1a1.KubernetesDeployMode{ + Type: &deployType, + }, + }, + } + ns = tc.infra.GetProxyInfra().Namespace + } + r := NewResourceRender(ns, cfg.DNSDomain, tc.infra.GetProxyInfra(), cfg.EnvoyGateway) + sa, err := r.ServiceAccount() require.NoError(t, err) + if test.OverrideTestData() { + deploymentYAML, err := yaml.Marshal(sa) + require.NoError(t, err) + // nolint: gosec + err = os.WriteFile(fmt.Sprintf("testdata/serviceaccount/%s.yaml", tc.name), deploymentYAML, 0o644) + require.NoError(t, err) + return + } + expected, err := loadServiceAccount(tc.name) require.NoError(t, err) diff --git a/internal/infrastructure/kubernetes/proxy/testdata/deployments/gateway-namespace-mode.yaml b/internal/infrastructure/kubernetes/proxy/testdata/deployments/gateway-namespace-mode.yaml index 46d64b61c9..2ac93cc680 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/deployments/gateway-namespace-mode.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/deployments/gateway-namespace-mode.yaml @@ -9,7 +9,7 @@ metadata: gateway.envoyproxy.io/owning-gateway-name: default gateway.envoyproxy.io/owning-gateway-namespace: default name: envoy-default-37a8eec1 - namespace: default + namespace: ns1 spec: progressDeadlineSeconds: 600 revisionHistoryLimit: 10 diff --git a/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/default.yaml b/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/default.yaml index 4e2731766a..04760e0b91 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/default.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/default.yaml @@ -1,10 +1,11 @@ apiVersion: v1 kind: ServiceAccount metadata: + creationTimestamp: null labels: - app.kubernetes.io/name: envoy app.kubernetes.io/component: proxy app.kubernetes.io/managed-by: envoy-gateway + app.kubernetes.io/name: envoy gateway.envoyproxy.io/owning-gateway-name: default gateway.envoyproxy.io/owning-gateway-namespace: default name: envoy-default-37a8eec1 diff --git a/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/gateway-namespace-mode.yaml b/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/gateway-namespace-mode.yaml new file mode 100644 index 0000000000..ac3b47bb81 --- /dev/null +++ b/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/gateway-namespace-mode.yaml @@ -0,0 +1,12 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + creationTimestamp: null + labels: + app.kubernetes.io/component: proxy + app.kubernetes.io/managed-by: envoy-gateway + app.kubernetes.io/name: envoy + gateway.envoyproxy.io/owning-gateway-name: default + gateway.envoyproxy.io/owning-gateway-namespace: default + name: envoy-default-37a8eec1 + namespace: ns1 diff --git a/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/with-annotations.yaml b/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/with-annotations.yaml index f63c97451c..f4076de4e1 100644 --- a/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/with-annotations.yaml +++ b/internal/infrastructure/kubernetes/proxy/testdata/serviceaccount/with-annotations.yaml @@ -4,10 +4,11 @@ metadata: annotations: anno1: value1 anno2: value2 + creationTimestamp: null labels: - app.kubernetes.io/name: envoy app.kubernetes.io/component: proxy app.kubernetes.io/managed-by: envoy-gateway + app.kubernetes.io/name: envoy gateway.envoyproxy.io/owning-gateway-name: default gateway.envoyproxy.io/owning-gateway-namespace: default name: envoy-default-37a8eec1 diff --git a/internal/infrastructure/kubernetes/proxy_infra.go b/internal/infrastructure/kubernetes/proxy_infra.go index 080271a6bd..f911e5273e 100644 --- a/internal/infrastructure/kubernetes/proxy_infra.go +++ b/internal/infrastructure/kubernetes/proxy_infra.go @@ -23,11 +23,8 @@ func (i *Infra) CreateOrUpdateProxyInfra(ctx context.Context, infra *ir.Infra) e return errors.New("infra proxy ir is nil") } - if i.EnvoyGateway.GatewayNamespaceMode() && i.Namespace == "" { - i.Namespace = infra.Proxy.Namespace - } - - r := proxy.NewResourceRender(i.Namespace, i.DNSDomain, infra.GetProxyInfra(), i.EnvoyGateway) + ns := i.GetResourceNamespace(infra) + r := proxy.NewResourceRender(ns, i.DNSDomain, infra.GetProxyInfra(), i.EnvoyGateway) return i.createOrUpdate(ctx, r) } @@ -37,10 +34,14 @@ func (i *Infra) DeleteProxyInfra(ctx context.Context, infra *ir.Infra) error { return errors.New("infra ir is nil") } - if i.EnvoyGateway.GatewayNamespaceMode() && i.Namespace == "" { - i.Namespace = infra.Proxy.Namespace - } - - r := proxy.NewResourceRender(i.Namespace, i.DNSDomain, infra.GetProxyInfra(), i.EnvoyGateway) + ns := i.GetResourceNamespace(infra) + r := proxy.NewResourceRender(ns, i.DNSDomain, infra.GetProxyInfra(), i.EnvoyGateway) return i.delete(ctx, r) } + +func (i *Infra) GetResourceNamespace(infra *ir.Infra) string { + if i.EnvoyGateway.GatewayNamespaceMode() { + return infra.Proxy.Namespace + } + return i.Namespace +} diff --git a/internal/infrastructure/kubernetes/ratelimit/resource_provider.go b/internal/infrastructure/kubernetes/ratelimit/resource_provider.go index 66219636ce..f379ccd757 100644 --- a/internal/infrastructure/kubernetes/ratelimit/resource_provider.go +++ b/internal/infrastructure/kubernetes/ratelimit/resource_provider.go @@ -38,8 +38,8 @@ const ( var statsConf string type ResourceRender struct { - // Namespace is the Namespace used for managed infra. - Namespace string + // ns is the Namespace used for managed infra. + ns string rateLimit *egv1a1.RateLimit rateLimitDeployment *egv1a1.KubernetesDeploymentSpec @@ -52,7 +52,7 @@ type ResourceRender struct { // NewResourceRender returns a new ResourceRender. func NewResourceRender(ns string, gateway *egv1a1.EnvoyGateway, ownerReferenceUID map[string]types.UID) *ResourceRender { return &ResourceRender{ - Namespace: ns, + ns: ns, rateLimit: gateway.RateLimit, rateLimitDeployment: gateway.GetEnvoyGatewayProvider().GetEnvoyGatewayKubeProvider().RateLimitDeployment, rateLimitHpa: gateway.GetEnvoyGatewayProvider().GetEnvoyGatewayKubeProvider().RateLimitHpa, @@ -64,6 +64,10 @@ func (r *ResourceRender) Name() string { return InfraName } +func (r *ResourceRender) Namespace() string { + return r.ns +} + func (r *ResourceRender) LabelSelector() labels.Selector { return labels.SelectorFromSet(rateLimitLabels()) } @@ -91,7 +95,7 @@ func (r *ResourceRender) ConfigMap(cert string) (*corev1.ConfigMap, error) { APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Name: "statsd-exporter-config", Labels: rateLimitLabels(), }, @@ -138,7 +142,7 @@ func (r *ResourceRender) Service() (*corev1.Service, error) { APIVersion: apiVersion, }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Name: InfraName, Labels: labels, }, @@ -171,7 +175,7 @@ func (r *ResourceRender) ServiceAccount() (*corev1.ServiceAccount, error) { APIVersion: apiVersion, }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Name: InfraName, }, } @@ -194,7 +198,7 @@ func (r *ResourceRender) ServiceAccount() (*corev1.ServiceAccount, error) { // Deployment returns the expected rate limit Deployment based on the provided infra. func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) { - containers := expectedRateLimitContainers(r.rateLimit, r.rateLimitDeployment, r.Namespace) + containers := expectedRateLimitContainers(r.rateLimit, r.rateLimitDeployment, r.Namespace()) selector := resource.GetSelector(rateLimitLabels()) podLabels := rateLimitLabels() @@ -227,7 +231,7 @@ func (r *ResourceRender) Deployment() (*appsv1.Deployment, error) { APIVersion: appsAPIVersion, }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Labels: rateLimitLabels(), }, Spec: appsv1.DeploymentSpec{ @@ -313,7 +317,7 @@ func (r *ResourceRender) HorizontalPodAutoscaler() (*autoscalingv2.HorizontalPod Kind: "HorizontalPodAutoscaler", }, ObjectMeta: metav1.ObjectMeta{ - Namespace: r.Namespace, + Namespace: r.Namespace(), Name: r.Name(), Labels: rateLimitLabels(), }, From f080b7f7da75a54386902c9276e24d5f6f48b86d Mon Sep 17 00:00:00 2001 From: zirain Date: Thu, 8 May 2025 17:00:23 +0800 Subject: [PATCH 2/4] nit Signed-off-by: zirain --- .../kubernetes/ratelimit/resource_provider.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/infrastructure/kubernetes/ratelimit/resource_provider.go b/internal/infrastructure/kubernetes/ratelimit/resource_provider.go index f379ccd757..9013127e0f 100644 --- a/internal/infrastructure/kubernetes/ratelimit/resource_provider.go +++ b/internal/infrastructure/kubernetes/ratelimit/resource_provider.go @@ -38,8 +38,8 @@ const ( var statsConf string type ResourceRender struct { - // ns is the Namespace used for managed infra. - ns string + // namespace is the Namespace used for managed infra. + namespace string rateLimit *egv1a1.RateLimit rateLimitDeployment *egv1a1.KubernetesDeploymentSpec @@ -52,7 +52,7 @@ type ResourceRender struct { // NewResourceRender returns a new ResourceRender. func NewResourceRender(ns string, gateway *egv1a1.EnvoyGateway, ownerReferenceUID map[string]types.UID) *ResourceRender { return &ResourceRender{ - ns: ns, + namespace: ns, rateLimit: gateway.RateLimit, rateLimitDeployment: gateway.GetEnvoyGatewayProvider().GetEnvoyGatewayKubeProvider().RateLimitDeployment, rateLimitHpa: gateway.GetEnvoyGatewayProvider().GetEnvoyGatewayKubeProvider().RateLimitHpa, @@ -65,7 +65,7 @@ func (r *ResourceRender) Name() string { } func (r *ResourceRender) Namespace() string { - return r.ns + return r.namespace } func (r *ResourceRender) LabelSelector() labels.Selector { From 0f57337c929005eb75766c121c3a49b15375a4ca Mon Sep 17 00:00:00 2001 From: zirain Date: Thu, 8 May 2025 17:28:47 +0800 Subject: [PATCH 3/4] nit Signed-off-by: zirain --- .../infrastructure/kubernetes/proxy/resource_provider.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider.go b/internal/infrastructure/kubernetes/proxy/resource_provider.go index 2fba10056f..0c8b5e16d6 100644 --- a/internal/infrastructure/kubernetes/proxy/resource_provider.go +++ b/internal/infrastructure/kubernetes/proxy/resource_provider.go @@ -46,7 +46,8 @@ const ( type ResourceRender struct { infra *ir.ProxyInfra - ns string + // namespace is the Namespace used for managed infra. + namespace string // DNSDomain is the dns domain used by k8s services. Defaults to "cluster.local". DNSDomain string @@ -58,7 +59,7 @@ type ResourceRender struct { func NewResourceRender(ns, dnsDomain string, infra *ir.ProxyInfra, gateway *egv1a1.EnvoyGateway) *ResourceRender { return &ResourceRender{ - ns: ns, + namespace: ns, DNSDomain: dnsDomain, infra: infra, ShutdownManager: gateway.GetEnvoyGatewayProvider().GetEnvoyGatewayKubeProvider().ShutdownManager, @@ -71,7 +72,7 @@ func (r *ResourceRender) Name() string { } func (r *ResourceRender) Namespace() string { - return r.ns + return r.namespace } func (r *ResourceRender) LabelSelector() labels.Selector { From b4698b51f74163a656c70ffdcdc74e7256d9da88 Mon Sep 17 00:00:00 2001 From: zirain Date: Thu, 8 May 2025 18:20:00 +0800 Subject: [PATCH 4/4] more nit Signed-off-by: zirain --- .../infrastructure/kubernetes/proxy/resource_provider_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go index f05f9d758a..88899754de 100644 --- a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go +++ b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go @@ -1334,10 +1334,10 @@ func TestServiceAccount(t *testing.T) { require.NoError(t, err) if test.OverrideTestData() { - deploymentYAML, err := yaml.Marshal(sa) + saYAML, err := yaml.Marshal(sa) require.NoError(t, err) // nolint: gosec - err = os.WriteFile(fmt.Sprintf("testdata/serviceaccount/%s.yaml", tc.name), deploymentYAML, 0o644) + err = os.WriteFile(fmt.Sprintf("testdata/serviceaccount/%s.yaml", tc.name), saYAML, 0o644) require.NoError(t, err) return }