Skip to content

fix: proxy creation/deletion error handling in GatewayNamespace mode #5954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zirain merged 4 commits into from
May 8, 2025
Merged

fix: proxy creation/deletion error handling in GatewayNamespace mode #5954

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
829dfd7
fix: proxy creation/deletion error handling in GatewayNamespace mode
zirain May 8, 2025
f080b7f
nit
zirain May 8, 2025
0f57337
nit
zirain May 8, 2025
b4698b5
more nit
zirain May 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 18 additions & 19 deletions internal/infrastructure/kubernetes/infra.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand All @@ -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
Expand Down
28 changes: 14 additions & 14 deletions internal/infrastructure/kubernetes/infra_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
}
)

Expand Down Expand Up @@ -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()),
}
)

Expand Down Expand Up @@ -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()),
}
)

Expand Down Expand Up @@ -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()),
}
)

Expand Down Expand Up @@ -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()),
}
)

Expand Down Expand Up @@ -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()),
}
)

Expand Down Expand Up @@ -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()),
}
)

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
27 changes: 15 additions & 12 deletions internal/infrastructure/kubernetes/proxy/resource_provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ const (
type ResourceRender struct {
infra *ir.ProxyInfra

// Namespace is the Namespace used for managed infra.
Namespace string
ns string
Comment thread
zirain marked this conversation as resolved.
Outdated

// DNSDomain is the dns domain used by k8s services. Defaults to "cluster.local".
DNSDomain string
Expand All @@ -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,
Expand All @@ -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)
}
Expand All @@ -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,
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand All @@ -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,
},
Expand Down Expand Up @@ -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
}
Expand All @@ -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,
},
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down
Loading