Merge pull request kubernetes-sigs#4734 from shuqz/ingress-plan-annotation

[feat i2g] Add IngressPlanAnnotation feature gate for dry-run plan

Author: k8s-ci-robot

controllers/ingress/dryrun.go

@@ -0,0 +1,50 @@
+package ingress
+
+import (
+	"context"
+	"fmt"
+
+	networking "k8s.io/api/networking/v1"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+const (
+	dryRunPlanAnnotation = annotations.AnnotationPrefixIngress + "/" + annotations.IngressSuffixDryRunPlan
+)
+
+// patchDryRunPlanAnnotation writes the serialized stack JSON to the dry-run-plan
+// annotation on the given ingress. It skips the patch if the value is unchanged
+// to avoid unnecessary API calls and reconcile loops.
+func patchDryRunPlanAnnotation(ctx context.Context, k8sClient client.Client, ing *networking.Ingress, planJSON string) error {
+	if ing.Annotations[dryRunPlanAnnotation] == planJSON {
+		return nil
+	}
+	ingOld := ing.DeepCopy()
+	if ing.Annotations == nil {
+		ing.Annotations = map[string]string{}
+	}
+	ing.Annotations[dryRunPlanAnnotation] = planJSON
+	if err := k8sClient.Patch(ctx, ing, client.MergeFrom(ingOld)); err != nil {
+		return fmt.Errorf("failed to patch dry-run plan annotation on ingress %s: %w", k8s.NamespacedName(ing), err)
+	}
+	return nil
+}
+
+// clearDryRunPlanAnnotation removes the dry-run-plan annotation from an ingress
+// if it's currently set. This is a no-op when the annotation is absent, so the
+// patch is skipped and no API call is made. Used by the group controller to
+// clean up stale plan annotations on non-primary members when the group's
+// membership shifts (so the migration console sees exactly one holder).
+func clearDryRunPlanAnnotation(ctx context.Context, k8sClient client.Client, ing *networking.Ingress) error {
+	if _, ok := ing.Annotations[dryRunPlanAnnotation]; !ok {
+		return nil
+	}
+	ingOld := ing.DeepCopy()
+	delete(ing.Annotations, dryRunPlanAnnotation)
+	if err := k8sClient.Patch(ctx, ing, client.MergeFrom(ingOld)); err != nil {
+		return fmt.Errorf("failed to clear dry-run plan annotation on ingress %s: %w", k8s.NamespacedName(ing), err)
+	}
+	return nil
+}

controllers/ingress/dryrun_test.go

@@ -0,0 +1,96 @@
+package ingress
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	networking "k8s.io/api/networking/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+func Test_patchDryRunPlanAnnotation(t *testing.T) {
+	tests := []struct {
+		name                 string
+		ingAnnotations       map[string]string
+		planJSON             string
+		wantAnnotation       string
+		wantOtherAnnotations map[string]string
+	}{
+		{
+			name:           "writes annotation when not present",
+			ingAnnotations: nil,
+			planJSON:       `{"id":"test-stack"}`,
+			wantAnnotation: `{"id":"test-stack"}`,
+		},
+		{
+			name: "skips patch when value unchanged",
+			ingAnnotations: map[string]string{
+				dryRunPlanAnnotation: `{"id":"test-stack"}`,
+			},
+			planJSON:       `{"id":"test-stack"}`,
+			wantAnnotation: `{"id":"test-stack"}`,
+		},
+		{
+			name: "updates annotation when value changed",
+			ingAnnotations: map[string]string{
+				dryRunPlanAnnotation: `{"id":"old-stack"}`,
+			},
+			planJSON:       `{"id":"new-stack"}`,
+			wantAnnotation: `{"id":"new-stack"}`,
+		},
+		{
+			name: "preserves existing annotations",
+			ingAnnotations: map[string]string{
+				"alb.ingress.kubernetes.io/scheme": "internet-facing",
+			},
+			planJSON:       `{"id":"test-stack"}`,
+			wantAnnotation: `{"id":"test-stack"}`,
+			wantOtherAnnotations: map[string]string{
+				"alb.ingress.kubernetes.io/scheme": "internet-facing",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ing := &networking.Ingress{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "test-ingress",
+					Namespace:   "default",
+					Annotations: tt.ingAnnotations,
+				},
+			}
+
+			scheme := runtime.NewScheme()
+			require.NoError(t, clientgoscheme.AddToScheme(scheme))
+
+			k8sClient := fake.NewClientBuilder().
+				WithScheme(scheme).
+				WithObjects(ing).
+				Build()
+
+			err := patchDryRunPlanAnnotation(context.Background(), k8sClient, ing, tt.planJSON)
+			require.NoError(t, err)
+
+			// Verify the annotation was persisted
+			updatedIng := &networking.Ingress{}
+			err = k8sClient.Get(context.Background(), types.NamespacedName{
+				Name:      "test-ingress",
+				Namespace: "default",
+			}, updatedIng)
+			require.NoError(t, err)
+			assert.Equal(t, tt.wantAnnotation, updatedIng.Annotations[dryRunPlanAnnotation])
+
+			// Verify other annotations were not clobbered
+			for k, v := range tt.wantOtherAnnotations {
+				assert.Equal(t, v, updatedIng.Annotations[k], "annotation %s should be preserved", k)
+			}
+		})
+	}
+}

controllers/ingress/group_controller.go

@@ -92,6 +92,7 @@ func NewGroupReconciler(cloud services.Cloud, k8sClient client.Client, eventReco
 
 		groupLoader:           groupLoader,
 		groupFinalizerManager: groupFinalizerManager,
+		featureGates:          controllerConfig.FeatureGates,
 		logger:                logger,
 		metricsCollector:      metricsCollector,
 		controllerName:        controllerName,
@@ -114,6 +115,7 @@ type groupReconciler struct {
 
 	groupLoader           ingress.GroupLoader
 	groupFinalizerManager ingress.FinalizerManager
+	featureGates          config.FeatureGates
 	logger                logr.Logger
 	metricsCollector      lbcmetrics.MetricCollector
 	controllerName        string
@@ -230,6 +232,24 @@ func (r *groupReconciler) buildAndDeployModel(ctx context.Context, ingGroup ingr
 	}
 	r.logger.Info("successfully built model", "model", stackJSON)
 
+	if r.featureGates.Enabled(config.IngressPlanAnnotation) && len(ingGroup.Members) > 0 {
+		if err := patchDryRunPlanAnnotation(ctx, r.k8sClient, ingGroup.Members[0].Ing, stackJSON); err != nil {
+			r.logger.Error(err, "failed to patch dry-run plan annotation", "ingress", k8s.NamespacedName(ingGroup.Members[0].Ing))
+		}
+		// Clear the dry-run-plan annotation from every non-primary member so a
+		// group that moves its holder across reconciles (e.g. a member with a
+		// lower group.order is added) doesn't leave stale plans behind. The
+		// migration console's discovery step errors out when it finds multiple
+		// ingresses in a group carrying the annotation — this cleanup keeps
+		// that invariant. We swallow individual failures to avoid blocking the
+		// main reconcile; the next pass retries.
+		for _, m := range ingGroup.Members[1:] {
+			if err := clearDryRunPlanAnnotation(ctx, r.k8sClient, m.Ing); err != nil {
+				r.logger.Error(err, "failed to clear stale dry-run plan annotation", "ingress", k8s.NamespacedName(m.Ing))
+			}
+		}
+	}
+
 	deployModelFn := func() {
 		err = r.stackDeployer.Deploy(ctx, stack, r.metricsCollector, "ingress")
 	}

docs/deploy/configurations.md

@@ -201,3 +201,4 @@ There are a set of key=value pairs that describe AWS load balancer controller fe
 | GatewayListenerSet                  | string                          | true         | Enable or disable the usage of ListenerSets in the Gateway API                                                                                                                                                                                                    |
 | ALBTargetControlAgent               | string                          | false        | Enable or disable the ALB Target Control Agent                                                                                                                                                                                                                    |
 | EnableCertificateManagement          | string                          | false        | Whether to enable the [Certificate Management feature](../guide/ingress/certificate_management.md).                                                                                            |
+| IngressPlanAnnotation                | string                          | false        | If enabled, the controller writes the serialized model stack JSON to the `alb.ingress.kubernetes.io/dry-run-plan` annotation on ingress. For grouped ingresses, the annotation is written to the first member (lowest group order). |

helm/aws-load-balancer-controller/values.yaml

@@ -428,6 +428,7 @@ controllerConfig:
   # ALBGatewayAPI: true
   # GatewayListenerSet: true
   # GlobalAcceleratorController: false
+  # IngressPlanAnnotation: false
   # EnhancedDefaultBehavior: false
   # EnableDefaultTagsLowPriority: false
   # ALBTargetControlAgent: false

pkg/annotations/constants.go

@@ -81,6 +81,7 @@ const (
 	IngressSuffixTargetControlPort                             = "target-control-port"
 	IngressSuffixCreateCertificate                             = "create-acm-cert"
 	IngressSuffixACMCaARN                                      = "acm-pca-arn"
+	IngressSuffixDryRunPlan                                    = "dry-run-plan"
 
 	// NLB annotation suffixes
 	// prefixes service.beta.kubernetes.io, service.kubernetes.io

pkg/config/feature_gates.go

@@ -40,6 +40,7 @@ const (
 	ALBTargetControlAgent         Feature = "ALBTargetControlAgent"
 	GatewayListenerSet            Feature = "GatewayListenerSet"
 	EnableCertificateManagement   Feature = "EnableCertificateManagement"
+	IngressPlanAnnotation         Feature = "IngressPlanAnnotation"
 )
 
 type FeatureGates interface {
@@ -94,6 +95,7 @@ func NewFeatureGates() FeatureGates {
 			ALBTargetControlAgent:         generateDefaultFeatureStatus(false),
 			GatewayListenerSet:            generateDefaultFeatureStatus(true),
 			EnableCertificateManagement:   generateDefaultFeatureStatus(false),
+			IngressPlanAnnotation:         generateDefaultFeatureStatus(false),
 		},
 	}
 }