Preserve eksctl commands correctness when user deletes subnets

integration/tests/managed/managed_nodegroup_test.go

@@ -11,9 +11,10 @@ import (
 	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
-	"github.com/aws/aws-sdk-go-v2/service/ec2"
+	awsec2 "github.com/aws/aws-sdk-go-v2/service/ec2"
 	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 	awseks "github.com/aws/aws-sdk-go-v2/service/eks"
+	ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -76,6 +77,7 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() {
 		ubuntuNodegroup          = "ng-ubuntu"
 		publicNodeGroup          = "ng-public"
 		privateNodeGroup         = "ng-private"
+		removedSubnetNodeGroup   = "ng-removed-subnet"
 	)
 
 	var (
@@ -488,14 +490,16 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() {
 		})
 	})
 
-	Context("eksctl utils update-cluster-vpc-config", Serial, func() {
-		makeAWSProvider := func(ctx context.Context, clusterConfig *api.ClusterConfig) api.ClusterProvider {
-			clusterProvider, err := eks.New(ctx, &api.ProviderConfig{Region: params.Region}, clusterConfig)
-			Expect(err).NotTo(HaveOccurred())
-			return clusterProvider.AWSProvider
-		}
-		getPrivateSubnetIDs := func(ctx context.Context, ec2API awsapi.EC2, vpcID string) []string {
-			out, err := ec2API.DescribeSubnets(ctx, &ec2.DescribeSubnetsInput{
+	Context("eksctl utils update-cluster-vpc-config", Serial, Ordered, func() {
+		var (
+			cluster                           *ekstypes.Cluster
+			clusterConfig                     *api.ClusterConfig
+			awsProvider                       api.ClusterProvider
+			publicSubnetIDs, privateSubnetIDs []string
+		)
+
+		getVPCSubnetIDs := func(ctx context.Context, ec2API awsapi.EC2, vpcID string) (publicIDs, privateIDs []string) {
+			out, err := ec2API.DescribeSubnets(ctx, &awsec2.DescribeSubnetsInput{
 				Filters: []ec2types.Filter{
 					{
 						Name:   aws.String("vpc-id"),
@@ -504,33 +508,46 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() {
 				},
 			})
 			Expect(err).NotTo(HaveOccurred())
-			var subnetIDs []string
 			for _, s := range out.Subnets {
-				if !*s.MapPublicIpOnLaunch {
-					subnetIDs = append(subnetIDs, *s.SubnetId)
+				if *s.MapPublicIpOnLaunch {
+					publicIDs = append(publicIDs, *s.SubnetId)
+					continue
 				}
+				privateIDs = append(privateIDs, *s.SubnetId)
 			}
-			return subnetIDs
+			return publicIDs, privateIDs
 		}
-		It("should update the VPC config", func() {
-			clusterConfig := makeClusterConfig()
+
+		BeforeAll(func() {
 			ctx := context.Background()
-			awsProvider := makeAWSProvider(ctx, clusterConfig)
-			cluster, err := awsProvider.EKS().DescribeCluster(ctx, &awseks.DescribeClusterInput{
+			clusterConfig = makeClusterConfig()
+
+			// initialize AWS provider
+			clusterProvider, err := eks.New(ctx, &api.ProviderConfig{Region: params.Region}, clusterConfig)
+			Expect(err).NotTo(HaveOccurred())
+			awsProvider = clusterProvider.AWSProvider
+
+			// get cluster subnets
+			output, err := awsProvider.EKS().DescribeCluster(ctx, &awseks.DescribeClusterInput{
 				Name: aws.String(params.ClusterName),
 			})
 			Expect(err).NotTo(HaveOccurred(), "error describing cluster")
-			clusterSubnetIDs := getPrivateSubnetIDs(ctx, awsProvider.EC2(), *cluster.Cluster.ResourcesVpcConfig.VpcId)
-			Expect(len(cluster.Cluster.ResourcesVpcConfig.SecurityGroupIds) > 0).To(BeTrue(), "at least one security group ID must be associated with the cluster")
+			cluster = output.Cluster
+			publicSubnetIDs, privateSubnetIDs = getVPCSubnetIDs(ctx, awsProvider.EC2(), *cluster.ResourcesVpcConfig.VpcId)
 
+			// enforce security group condition
+			Expect(len(cluster.ResourcesVpcConfig.SecurityGroupIds) > 0).To(BeTrue(), "at least one security group ID must be associated with the cluster")
+		})
+
+		It("should update the VPC config", func() {
 			clusterVPC := &api.ClusterVPC{
 				ClusterEndpoints: &api.ClusterEndpoints{
 					PrivateAccess: api.Enabled(),
 					PublicAccess:  api.Enabled(),
 				},
 				PublicAccessCIDRs:            []string{"127.0.0.1/32"},
-				ControlPlaneSubnetIDs:        clusterSubnetIDs,
-				ControlPlaneSecurityGroupIDs: []string{cluster.Cluster.ResourcesVpcConfig.SecurityGroupIds[0]},
+				ControlPlaneSubnetIDs:        publicSubnetIDs,
+				ControlPlaneSecurityGroupIDs: []string{cluster.ResourcesVpcConfig.SecurityGroupIds[0]},
 			}
 			By("accepting CLI options")
 			cmd := params.EksctlUtilsCmd.WithArgs(
@@ -587,6 +604,32 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() {
 			)
 			Expect(cmd).To(RunSuccessfully())
 		})
+
+		It("should support creating nodegroups after deleting a removed control plane subnet", func() {
+			_, err := awsProvider.EC2().DeleteSubnet(context.Background(), &awsec2.DeleteSubnetInput{SubnetId: &privateSubnetIDs[0]})
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("should fail early if all private subnets were deleted and privateNetworking is set to true", func() {
+			clusterConfig.ManagedNodeGroups = append(clusterConfig.ManagedNodeGroups, &api.ManagedNodeGroup{
+				NodeGroupBase: &api.NodeGroupBase{
+					Name:              removedSubnetNodeGroup,
+					PrivateNetworking: true,
+				},
+			})
+			cmd := params.EksctlCreateCmd.WithArgs(
+				"nodegroup",
+				"--config-file", "-",
+			).
+				WithoutArg("--region", params.Region).
+				WithStdin(clusterutils.Reader(clusterConfig))
+			session := cmd.Run()
+			Expect(session.ExitCode()).To(Equal(1))
+			Expect(strings.Split(string(session.Buffer().Contents()), "\n")).To(ContainElements(
+				ContainSubstring(fmt.Sprintf("%s was found on cluster cloudformation stack, but has been removed from %s outside of eksctl", privateSubnetIDs[0], *cluster.ResourcesVpcConfig.VpcId)),
+				ContainSubstring(fmt.Sprintf("cannot have privateNetworking:true for nodegroup %s, since no private subnets were found within %s", removedSubnetNodeGroup, *cluster.ResourcesVpcConfig.VpcId)),
+			))
+		})
 	})
 })
 

pkg/actions/nodegroup/create.go

@@ -135,6 +135,10 @@ func (m *Manager) Create(ctx context.Context, options CreateOpts, nodegroupFilte
 		}
 	}
 
+	if err := validateSubnetsAvailability(cfg); err != nil {
+		return err
+	}
+
 	if err := vpc.ValidateLegacySubnetsForNodeGroups(ctx, cfg, ctl.AWSProvider); err != nil {
 		return err
 	}
@@ -404,3 +408,52 @@ func validateSecurityGroup(ctx context.Context, ec2API awsapi.EC2, securityGroup
 	}
 	return hasDefaultEgressRule, nil
 }
+
+func validateSubnetsAvailability(spec *api.ClusterConfig) error {
+	validateSubnetsAvailabilityForNg := func(np api.NodePool) error {
+		ng := np.BaseNodeGroup()
+		subnetTypeForPrivateNetworking := map[bool]string{
+			true:  "private",
+			false: "public",
+		}
+		unavailableSubnetsErr := func(subnetLocation string) error {
+			return fmt.Errorf("cannot have privateNetworking:%t for nodegroup %s, since no %s subnets were found within %s",
+				ng.PrivateNetworking, ng.Name, subnetTypeForPrivateNetworking[ng.PrivateNetworking], subnetLocation)
+		}
+
+		// don't check private networking compatibility for:
+		// self-managed nodegroups on local zones
+		if nodeGroup, ok := np.(*api.NodeGroup); (ok && len(nodeGroup.LocalZones) > 0) ||
+			// nodegroups on outposts
+			(ng.OutpostARN != "" || spec.IsControlPlaneOnOutposts()) ||
+			// nodegroups on user specified subnets
+			len(ng.Subnets) > 0 {
+			return nil
+		}
+		shouldCheckAcrossAllAZs := true
+		for _, az := range ng.AvailabilityZones {
+			shouldCheckAcrossAllAZs = false
+			if _, ok := spec.VPC.Subnets.Private[az]; !ok && ng.PrivateNetworking {
+				return unavailableSubnetsErr(az)
+			}
+			if _, ok := spec.VPC.Subnets.Public[az]; !ok && !ng.PrivateNetworking {
+				return unavailableSubnetsErr(az)
+			}
+		}
+		if shouldCheckAcrossAllAZs {
+			if ng.PrivateNetworking && len(spec.VPC.Subnets.Private) == 0 {
+				return unavailableSubnetsErr(spec.VPC.ID)
+			}
+			if !ng.PrivateNetworking && len(spec.VPC.Subnets.Public) == 0 {
+				return unavailableSubnetsErr(spec.VPC.ID)
+			}
+		}
+		return nil
+	}
+	for _, np := range nodes.ToNodePools(spec) {
+		if err := validateSubnetsAvailabilityForNg(np); err != nil {
+			return err
+		}
+	}
+	return nil
+}