From b0d7c2bd2c60a94be28c2f017d6c5a93a8cc4285 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Tue, 5 Oct 2021 15:05:32 +0200 Subject: [PATCH 01/12] Check if managed addons are set when ipv6 is enabled --- examples/29-vpc-with-ip-family.yaml | 5 ++++ pkg/apis/eksctl.io/v1alpha5/validation.go | 24 +++++++++++++++++++ .../eksctl.io/v1alpha5/validation_test.go | 16 +++++++++++++ userdocs/src/usage/vpc-networking.md | 7 +++++- 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/examples/29-vpc-with-ip-family.yaml b/examples/29-vpc-with-ip-family.yaml index 78792bf93a..e093d7f885 100644 --- a/examples/29-vpc-with-ip-family.yaml +++ b/examples/29-vpc-with-ip-family.yaml @@ -11,4 +11,9 @@ metadata: vpc: ipFamily: IPv6 +addons: + - name: vpc-cni + - name: coredns + - name: kube-proxy + managedNodeGroups: [] diff --git a/pkg/apis/eksctl.io/v1alpha5/validation.go b/pkg/apis/eksctl.io/v1alpha5/validation.go index d22c75ac34..03f97cf2a5 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation.go @@ -169,10 +169,34 @@ func (c *ClusterConfig) ValidateVPCConfig() error { if *v != string(IPV4Family) && *v != string(IPV6Family) { return fmt.Errorf("invalid value %s for ipFamily; allowed are %s and %s", *v, IPV4Family, IPV6Family) } + // This is the new vpc check, I need this check when the user sets it. + if *v == string(IPV6Family) { + if missing := c.addonContainsManagedAddons([]string{"vpc-cni", "coredns", "kube-proxy"}); len(missing) != 0 { + return fmt.Errorf("managed addons must be defined in case of IPv6; missing addon(s): %s", strings.Join(missing, ", ")) + } + } } return nil } +// addonContainsManagedAddons finds managed addons in the config and returns those it couldn't find. +func (c *ClusterConfig) addonContainsManagedAddons(addons []string) []string { + var missing []string + for _, a := range addons { + found := false + for _, add := range c.Addons { + if strings.ToLower(add.Name) == a { + found = true + break + } + } + if !found { + missing = append(missing, a) + } + } + return missing +} + // ValidateClusterEndpointConfig checks the endpoint configuration for potential issues func (c *ClusterConfig) ValidateClusterEndpointConfig() error { if !c.HasClusterEndpointAccess() { diff --git a/pkg/apis/eksctl.io/v1alpha5/validation_test.go b/pkg/apis/eksctl.io/v1alpha5/validation_test.go index 269097a8b0..d763a1df40 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation_test.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation_test.go @@ -546,10 +546,26 @@ var _ = Describe("ClusterConfig validation", func() { It("accepts that setting", func() { ipv6 := string(api.IPV6Family) cfg.VPC.IPFamily = &ipv6 + cfg.Addons = append(cfg.Addons, + &api.Addon{Name: "kube-proxy"}, + &api.Addon{Name: "coredns"}, + &api.Addon{Name: "vpc-cni"}, + ) err = cfg.ValidateVPCConfig() Expect(err).ToNot(HaveOccurred()) }) }) + When("ipFamily is set ot IPv6 but no managed addons are provided", func() { + It("it returns an error including which addons are missing", func() { + ipv6 := string(api.IPV6Family) + cfg.VPC.IPFamily = &ipv6 + cfg.Addons = append(cfg.Addons, + &api.Addon{Name: "kube-proxy"}, + ) + err = cfg.ValidateVPCConfig() + Expect(err).To(MatchError(ContainSubstring("managed addons must be defined in case of IPv6; missing addon(s): vpc-cni, coredns"))) + }) + }) When("ipFamily isn't IPv4 or IPv6", func() { It("returns an error", func() { invalid := "invalid" diff --git a/userdocs/src/usage/vpc-networking.md b/userdocs/src/usage/vpc-networking.md index fe861a7462..bfb773a3d4 100644 --- a/userdocs/src/usage/vpc-networking.md +++ b/userdocs/src/usage/vpc-networking.md @@ -40,9 +40,14 @@ metadata: vpc: ipFamily: IPv6 # or IPv4 + +addons: + - name: vpc-cni + - name: coredns + - name: kube-proxy ``` -This is an in config file setting only. The default value is `IPv4`. +This is an in config file setting only and managed addons need to be defined when IPv6 is set. The default value is `IPv4`. ## Change VPC CIDR From f3e3d1a17afe67fde8895249ed94c087ae652b8b Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Tue, 5 Oct 2021 15:47:31 +0200 Subject: [PATCH 02/12] Add OIDC check for ipv6 cluster --- examples/29-vpc-with-ip-family.yaml | 3 ++ pkg/apis/eksctl.io/v1alpha5/validation.go | 3 ++ .../eksctl.io/v1alpha5/validation_test.go | 35 +++++++++++++++++++ userdocs/src/usage/vpc-networking.md | 6 +++- 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/examples/29-vpc-with-ip-family.yaml b/examples/29-vpc-with-ip-family.yaml index e093d7f885..65a07b5c2f 100644 --- a/examples/29-vpc-with-ip-family.yaml +++ b/examples/29-vpc-with-ip-family.yaml @@ -16,4 +16,7 @@ addons: - name: coredns - name: kube-proxy +iam: + withOIDC: true + managedNodeGroups: [] diff --git a/pkg/apis/eksctl.io/v1alpha5/validation.go b/pkg/apis/eksctl.io/v1alpha5/validation.go index 03f97cf2a5..467c040ee8 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation.go @@ -174,6 +174,9 @@ func (c *ClusterConfig) ValidateVPCConfig() error { if missing := c.addonContainsManagedAddons([]string{"vpc-cni", "coredns", "kube-proxy"}); len(missing) != 0 { return fmt.Errorf("managed addons must be defined in case of IPv6; missing addon(s): %s", strings.Join(missing, ", ")) } + if c.IAM == nil || c.IAM != nil && !IsEnabled(c.IAM.WithOIDC) { + return fmt.Errorf("oidc needs to be enabled if IPv6 is set") + } } } return nil diff --git a/pkg/apis/eksctl.io/v1alpha5/validation_test.go b/pkg/apis/eksctl.io/v1alpha5/validation_test.go index d763a1df40..dfb3e30210 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation_test.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation_test.go @@ -551,6 +551,9 @@ var _ = Describe("ClusterConfig validation", func() { &api.Addon{Name: "coredns"}, &api.Addon{Name: "vpc-cni"}, ) + cfg.IAM = &api.ClusterIAM{ + WithOIDC: api.Enabled(), + } err = cfg.ValidateVPCConfig() Expect(err).ToNot(HaveOccurred()) }) @@ -559,6 +562,9 @@ var _ = Describe("ClusterConfig validation", func() { It("it returns an error including which addons are missing", func() { ipv6 := string(api.IPV6Family) cfg.VPC.IPFamily = &ipv6 + cfg.IAM = &api.ClusterIAM{ + WithOIDC: api.Enabled(), + } cfg.Addons = append(cfg.Addons, &api.Addon{Name: "kube-proxy"}, ) @@ -566,6 +572,35 @@ var _ = Describe("ClusterConfig validation", func() { Expect(err).To(MatchError(ContainSubstring("managed addons must be defined in case of IPv6; missing addon(s): vpc-cni, coredns"))) }) }) + When("iam is not set", func() { + It("returns an error", func() { + ipv6 := string(api.IPV6Family) + cfg.VPC.IPFamily = &ipv6 + cfg.Addons = append(cfg.Addons, + &api.Addon{Name: "kube-proxy"}, + &api.Addon{Name: "coredns"}, + &api.Addon{Name: "vpc-cni"}, + ) + err = cfg.ValidateVPCConfig() + Expect(err).To(MatchError(ContainSubstring("oidc needs to be enabled if IPv6 is set"))) + }) + }) + When("iam is set but OIDC is disabled", func() { + It("returns an error", func() { + ipv6 := string(api.IPV6Family) + cfg.VPC.IPFamily = &ipv6 + cfg.IAM = &api.ClusterIAM{ + WithOIDC: api.Disabled(), + } + cfg.Addons = append(cfg.Addons, + &api.Addon{Name: "kube-proxy"}, + &api.Addon{Name: "coredns"}, + &api.Addon{Name: "vpc-cni"}, + ) + err = cfg.ValidateVPCConfig() + Expect(err).To(MatchError(ContainSubstring("oidc needs to be enabled if IPv6 is set"))) + }) + }) When("ipFamily isn't IPv4 or IPv6", func() { It("returns an error", func() { invalid := "invalid" diff --git a/userdocs/src/usage/vpc-networking.md b/userdocs/src/usage/vpc-networking.md index bfb773a3d4..8a3b97fcc3 100644 --- a/userdocs/src/usage/vpc-networking.md +++ b/userdocs/src/usage/vpc-networking.md @@ -45,9 +45,13 @@ addons: - name: vpc-cni - name: coredns - name: kube-proxy + +iam: + withOIDC: true ``` -This is an in config file setting only and managed addons need to be defined when IPv6 is set. The default value is `IPv4`. +This is an in config file setting only. Managed addons need to be defined when IPv6 is set along with OIDC. +The default value is `IPv4`. ## Change VPC CIDR From 46668695b8e2c746538eeea3a050dd47375a22f5 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Tue, 5 Oct 2021 16:49:31 +0200 Subject: [PATCH 03/12] Add version check if ipv6 is defined --- examples/29-vpc-with-ip-family.yaml | 1 + pkg/apis/eksctl.io/v1alpha5/validation.go | 17 ++++++++++----- .../eksctl.io/v1alpha5/validation_test.go | 21 +++++++++++++++++++ pkg/ctl/cmdutils/configfile_test.go | 2 +- pkg/utils/utils.go | 2 +- userdocs/src/usage/vpc-networking.md | 8 ++++++- 6 files changed, 43 insertions(+), 8 deletions(-) diff --git a/examples/29-vpc-with-ip-family.yaml b/examples/29-vpc-with-ip-family.yaml index 65a07b5c2f..8d2e9ba73f 100644 --- a/examples/29-vpc-with-ip-family.yaml +++ b/examples/29-vpc-with-ip-family.yaml @@ -7,6 +7,7 @@ kind: ClusterConfig metadata: name: cluster-2 region: eu-north-1 + version: "1.21" vpc: ipFamily: IPv6 diff --git a/pkg/apis/eksctl.io/v1alpha5/validation.go b/pkg/apis/eksctl.io/v1alpha5/validation.go index 467c040ee8..e112d84af1 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation.go @@ -14,6 +14,7 @@ import ( corev1 "k8s.io/api/core/v1" + "github.com/weaveworks/eksctl/pkg/utils" "github.com/weaveworks/eksctl/pkg/utils/taints" "k8s.io/apimachinery/pkg/util/validation" @@ -138,11 +139,6 @@ func ValidateClusterConfig(cfg *ClusterConfig) error { return errors.New("field secretsEncryption.keyARN is required for enabling secrets encryption") } - // manageSharedNodeSecurityGroupRules cannot be disabled if using eksctl managed security groups - if cfg.VPC != nil && cfg.VPC.SharedNodeSecurityGroup == "" && IsDisabled(cfg.VPC.ManageSharedNodeSecurityGroupRules) { - return errors.New("vpc.manageSharedNodeSecurityGroupRules must be enabled when using ekstcl-managed security groups") - } - return nil } @@ -177,8 +173,19 @@ func (c *ClusterConfig) ValidateVPCConfig() error { if c.IAM == nil || c.IAM != nil && !IsEnabled(c.IAM.WithOIDC) { return fmt.Errorf("oidc needs to be enabled if IPv6 is set") } + + if version, err := utils.CompareVersions(c.Metadata.Version, Version1_21); err != nil { + return fmt.Errorf("failed to convert %s cluster version to semver: %w", c.Metadata.Version, err) + } else if err == nil && version == -1 { + return fmt.Errorf("cluster version must be >= %s", Version1_21) + } } } + + // manageSharedNodeSecurityGroupRules cannot be disabled if using eksctl managed security groups + if c.VPC.SharedNodeSecurityGroup == "" && IsDisabled(c.VPC.ManageSharedNodeSecurityGroupRules) { + return errors.New("vpc.manageSharedNodeSecurityGroupRules must be enabled when using ekstcl-managed security groups") + } return nil } diff --git a/pkg/apis/eksctl.io/v1alpha5/validation_test.go b/pkg/apis/eksctl.io/v1alpha5/validation_test.go index dfb3e30210..fb5eb256b7 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation_test.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation_test.go @@ -554,10 +554,31 @@ var _ = Describe("ClusterConfig validation", func() { cfg.IAM = &api.ClusterIAM{ WithOIDC: api.Enabled(), } + cfg.Metadata.Version = api.Version1_21 err = cfg.ValidateVPCConfig() Expect(err).ToNot(HaveOccurred()) }) }) + When("ipFamily is set ot IPv6 but version is not or too low", func() { + It("returns an error", func() { + ipv6 := string(api.IPV6Family) + cfg.VPC.IPFamily = &ipv6 + cfg.Addons = append(cfg.Addons, + &api.Addon{Name: "kube-proxy"}, + &api.Addon{Name: "coredns"}, + &api.Addon{Name: "vpc-cni"}, + ) + cfg.IAM = &api.ClusterIAM{ + WithOIDC: api.Enabled(), + } + cfg.Metadata.Version = "" + err = cfg.ValidateVPCConfig() + Expect(err).To(MatchError(ContainSubstring("failed to convert cluster version to semver: unable to parse first version"))) + cfg.Metadata.Version = api.Version1_12 + err = cfg.ValidateVPCConfig() + Expect(err).To(MatchError(ContainSubstring("cluster version must be >= 1.21"))) + }) + }) When("ipFamily is set ot IPv6 but no managed addons are provided", func() { It("it returns an error including which addons are missing", func() { ipv6 := string(api.IPV6Family) diff --git a/pkg/ctl/cmdutils/configfile_test.go b/pkg/ctl/cmdutils/configfile_test.go index 6afd08c98c..93281c3542 100644 --- a/pkg/ctl/cmdutils/configfile_test.go +++ b/pkg/ctl/cmdutils/configfile_test.go @@ -156,9 +156,9 @@ var _ = Describe("cmdutils configfile", func() { } err := NewMetadataLoader(cmd).Load() + Expect(err).ToNot(HaveOccurred()) cfg := cmd.ClusterConfig - Expect(err).ToNot(HaveOccurred()) Expect(cfg.Metadata.Name).ToNot(BeEmpty()) Expect(cfg.Metadata.Region).ToNot(BeEmpty()) Expect(cfg.Metadata.Region).To(Equal(cmd.ProviderConfig.Region)) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 1de713e33b..bfe395d887 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -62,7 +62,7 @@ func IsMinVersion(minimumVersion, version string) (bool, error) { // CompareVersions compares two version strings with the usual conventions: // returns 0 if a == b // returns 1 if a > b -// returns -1 if b < a +// returns -1 if a < b func CompareVersions(a, b string) (int, error) { aVersion, err := semver.ParseTolerant(a) if err != nil { diff --git a/userdocs/src/usage/vpc-networking.md b/userdocs/src/usage/vpc-networking.md index 8a3b97fcc3..dc3d415127 100644 --- a/userdocs/src/usage/vpc-networking.md +++ b/userdocs/src/usage/vpc-networking.md @@ -37,6 +37,7 @@ kind: ClusterConfig metadata: name: my-test region: us-west-2 + version: "1.21" vpc: ipFamily: IPv6 # or IPv4 @@ -50,7 +51,12 @@ iam: withOIDC: true ``` -This is an in config file setting only. Managed addons need to be defined when IPv6 is set along with OIDC. +This is an in config file setting only. When IPv6 is set, the following restriction must be followed: + +- OIDC is enabled +- managed addons are defined as shows above +- version must be => 1.21 + The default value is `IPv4`. ## Change VPC CIDR From ebe72a7ee61d62b312454b13870a1a0d6bd64a21 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Wed, 6 Oct 2021 12:50:30 +0200 Subject: [PATCH 04/12] Setting vpc.NAT is not allowed with ipv6 --- pkg/apis/eksctl.io/v1alpha5/validation.go | 4 ++++ .../eksctl.io/v1alpha5/validation_test.go | 21 +++++++++++++++++++ userdocs/src/usage/vpc-networking.md | 1 + 3 files changed, 26 insertions(+) diff --git a/pkg/apis/eksctl.io/v1alpha5/validation.go b/pkg/apis/eksctl.io/v1alpha5/validation.go index e112d84af1..237ecb07a5 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation.go @@ -179,6 +179,10 @@ func (c *ClusterConfig) ValidateVPCConfig() error { } else if err == nil && version == -1 { return fmt.Errorf("cluster version must be >= %s", Version1_21) } + + if c.VPC.NAT != nil { + return fmt.Errorf("setting NAT is not supported with IPv6") + } } } diff --git a/pkg/apis/eksctl.io/v1alpha5/validation_test.go b/pkg/apis/eksctl.io/v1alpha5/validation_test.go index fb5eb256b7..cb1f8ebec4 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation_test.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation_test.go @@ -545,6 +545,7 @@ var _ = Describe("ClusterConfig validation", func() { When("ipFamily is set ot IPv6", func() { It("accepts that setting", func() { ipv6 := string(api.IPV6Family) + cfg.VPC.NAT = nil cfg.VPC.IPFamily = &ipv6 cfg.Addons = append(cfg.Addons, &api.Addon{Name: "kube-proxy"}, @@ -563,6 +564,7 @@ var _ = Describe("ClusterConfig validation", func() { It("returns an error", func() { ipv6 := string(api.IPV6Family) cfg.VPC.IPFamily = &ipv6 + cfg.VPC.NAT = nil cfg.Addons = append(cfg.Addons, &api.Addon{Name: "kube-proxy"}, &api.Addon{Name: "coredns"}, @@ -582,6 +584,7 @@ var _ = Describe("ClusterConfig validation", func() { When("ipFamily is set ot IPv6 but no managed addons are provided", func() { It("it returns an error including which addons are missing", func() { ipv6 := string(api.IPV6Family) + cfg.VPC.NAT = nil cfg.VPC.IPFamily = &ipv6 cfg.IAM = &api.ClusterIAM{ WithOIDC: api.Enabled(), @@ -630,6 +633,24 @@ var _ = Describe("ClusterConfig validation", func() { Expect(err).To(MatchError(ContainSubstring("invalid value invalid for ipFamily; allowed are IPv4 and IPv6"))) }) }) + When("ipFamily is set to IPv6 and vpc.NAT is defined", func() { + It("it returns an error", func() { + ipv6 := string(api.IPV6Family) + cfg.VPC.IPFamily = &ipv6 + cfg.Metadata.Version = api.Version1_22 + cfg.IAM = &api.ClusterIAM{ + WithOIDC: api.Enabled(), + } + cfg.Addons = append(cfg.Addons, + &api.Addon{Name: "kube-proxy"}, + &api.Addon{Name: "coredns"}, + &api.Addon{Name: "vpc-cni"}, + ) + cfg.VPC.NAT = &api.ClusterNAT{} + err = cfg.ValidateVPCConfig() + Expect(err).To(MatchError(ContainSubstring("setting NAT is not supported with IPv6"))) + }) + }) }) Context("CIDRs", func() { diff --git a/userdocs/src/usage/vpc-networking.md b/userdocs/src/usage/vpc-networking.md index dc3d415127..3a0a80e15c 100644 --- a/userdocs/src/usage/vpc-networking.md +++ b/userdocs/src/usage/vpc-networking.md @@ -56,6 +56,7 @@ This is an in config file setting only. When IPv6 is set, the following restrict - OIDC is enabled - managed addons are defined as shows above - version must be => 1.21 +- setting vpc.NAT is not allowed The default value is `IPv4`. From 9fdc6679bcbe21695b295ac51365854eda209ddb Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Wed, 6 Oct 2021 15:01:52 +0200 Subject: [PATCH 05/12] serviceIPv4CIDR is not supported with ipv6 --- examples/29-vpc-with-ip-family.yaml | 2 +- pkg/apis/eksctl.io/v1alpha5/validation.go | 4 ++++ .../eksctl.io/v1alpha5/validation_test.go | 21 +++++++++++++++++++ userdocs/src/usage/vpc-networking.md | 1 + 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/examples/29-vpc-with-ip-family.yaml b/examples/29-vpc-with-ip-family.yaml index 8d2e9ba73f..b1824ffa25 100644 --- a/examples/29-vpc-with-ip-family.yaml +++ b/examples/29-vpc-with-ip-family.yaml @@ -15,7 +15,7 @@ vpc: addons: - name: vpc-cni - name: coredns - - name: kube-proxy + - name: kube-proxy iam: withOIDC: true diff --git a/pkg/apis/eksctl.io/v1alpha5/validation.go b/pkg/apis/eksctl.io/v1alpha5/validation.go index 237ecb07a5..cd444ee4d5 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation.go @@ -183,6 +183,10 @@ func (c *ClusterConfig) ValidateVPCConfig() error { if c.VPC.NAT != nil { return fmt.Errorf("setting NAT is not supported with IPv6") } + + if c.KubernetesNetworkConfig != nil && c.KubernetesNetworkConfig.ServiceIPv4CIDR != "" { + return fmt.Errorf("service ipv4 cidr is not supported with IPv6") + } } } diff --git a/pkg/apis/eksctl.io/v1alpha5/validation_test.go b/pkg/apis/eksctl.io/v1alpha5/validation_test.go index cb1f8ebec4..a085bfb1a9 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation_test.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation_test.go @@ -651,6 +651,27 @@ var _ = Describe("ClusterConfig validation", func() { Expect(err).To(MatchError(ContainSubstring("setting NAT is not supported with IPv6"))) }) }) + When("ipFamily is set to IPv6 and serviceIPv4CIDR is not empty", func() { + It("it returns an error", func() { + ipv6 := string(api.IPV6Family) + cfg.VPC.IPFamily = &ipv6 + cfg.Metadata.Version = api.Version1_22 + cfg.IAM = &api.ClusterIAM{ + WithOIDC: api.Enabled(), + } + cfg.Addons = append(cfg.Addons, + &api.Addon{Name: "kube-proxy"}, + &api.Addon{Name: "coredns"}, + &api.Addon{Name: "vpc-cni"}, + ) + cfg.KubernetesNetworkConfig = &api.KubernetesNetworkConfig{ + ServiceIPv4CIDR: "192.168.0.0/24", + } + cfg.VPC.NAT = nil + err = cfg.ValidateVPCConfig() + Expect(err).To(MatchError(ContainSubstring("service ipv4 cidr is not supported with IPv6"))) + }) + }) }) Context("CIDRs", func() { diff --git a/userdocs/src/usage/vpc-networking.md b/userdocs/src/usage/vpc-networking.md index 3a0a80e15c..10fae907a8 100644 --- a/userdocs/src/usage/vpc-networking.md +++ b/userdocs/src/usage/vpc-networking.md @@ -57,6 +57,7 @@ This is an in config file setting only. When IPv6 is set, the following restrict - managed addons are defined as shows above - version must be => 1.21 - setting vpc.NAT is not allowed +- serviceIPv4CIDR is not supported together with IPv6 The default value is `IPv4`. From cbb0d74f7fb2344a2336862c18ee7786ec654893 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Wed, 6 Oct 2021 15:53:24 +0200 Subject: [PATCH 06/12] AutoAllocateIPv6 is not supported together with ipv6 --- pkg/apis/eksctl.io/v1alpha5/validation.go | 4 +++ .../eksctl.io/v1alpha5/validation_test.go | 31 +++++++++++++++---- userdocs/src/usage/vpc-networking.md | 1 + 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/pkg/apis/eksctl.io/v1alpha5/validation.go b/pkg/apis/eksctl.io/v1alpha5/validation.go index de9e90893d..55fd50ab47 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation.go @@ -187,6 +187,10 @@ func (c *ClusterConfig) ValidateVPCConfig() error { if c.KubernetesNetworkConfig != nil && c.KubernetesNetworkConfig.ServiceIPv4CIDR != "" { return fmt.Errorf("service ipv4 cidr is not supported with IPv6") } + + if IsEnabled(c.VPC.AutoAllocateIPv6) { + return fmt.Errorf("auto allocate ipv6 is not supported with IPv6") + } } } diff --git a/pkg/apis/eksctl.io/v1alpha5/validation_test.go b/pkg/apis/eksctl.io/v1alpha5/validation_test.go index 61d4c68c13..f9e6e509d5 100644 --- a/pkg/apis/eksctl.io/v1alpha5/validation_test.go +++ b/pkg/apis/eksctl.io/v1alpha5/validation_test.go @@ -640,9 +640,9 @@ var _ = Describe("ClusterConfig validation", func() { WithOIDC: api.Enabled(), } cfg.Addons = append(cfg.Addons, - &api.Addon{Name: "kube-proxy"}, - &api.Addon{Name: "coredns"}, - &api.Addon{Name: "vpc-cni"}, + &api.Addon{Name: api.KubeProxyAddon}, + &api.Addon{Name: api.CoreDNSAddon}, + &api.Addon{Name: api.VPCCNIAddon}, ) cfg.VPC.NAT = &api.ClusterNAT{} err = cfg.ValidateVPCConfig() @@ -658,9 +658,9 @@ var _ = Describe("ClusterConfig validation", func() { WithOIDC: api.Enabled(), } cfg.Addons = append(cfg.Addons, - &api.Addon{Name: "kube-proxy"}, - &api.Addon{Name: "coredns"}, - &api.Addon{Name: "vpc-cni"}, + &api.Addon{Name: api.KubeProxyAddon}, + &api.Addon{Name: api.CoreDNSAddon}, + &api.Addon{Name: api.VPCCNIAddon}, ) cfg.KubernetesNetworkConfig = &api.KubernetesNetworkConfig{ ServiceIPv4CIDR: "192.168.0.0/24", @@ -670,6 +670,25 @@ var _ = Describe("ClusterConfig validation", func() { Expect(err).To(MatchError(ContainSubstring("service ipv4 cidr is not supported with IPv6"))) }) }) + When("ipFamily is set to IPv6 and AutoAllocateIPv6 is set", func() { + It("it returns an error", func() { + ipv6 := string(api.IPV6Family) + cfg.VPC.IPFamily = &ipv6 + cfg.VPC.AutoAllocateIPv6 = api.Enabled() + cfg.Metadata.Version = api.Version1_22 + cfg.IAM = &api.ClusterIAM{ + WithOIDC: api.Enabled(), + } + cfg.Addons = append(cfg.Addons, + &api.Addon{Name: api.KubeProxyAddon}, + &api.Addon{Name: api.CoreDNSAddon}, + &api.Addon{Name: api.VPCCNIAddon}, + ) + cfg.VPC.NAT = nil + err = cfg.ValidateVPCConfig() + Expect(err).To(MatchError(ContainSubstring("auto allocate ipv6 is not supported with IPv6"))) + }) + }) }) Context("CIDRs", func() { diff --git a/userdocs/src/usage/vpc-networking.md b/userdocs/src/usage/vpc-networking.md index 10fae907a8..cea55a23a7 100644 --- a/userdocs/src/usage/vpc-networking.md +++ b/userdocs/src/usage/vpc-networking.md @@ -58,6 +58,7 @@ This is an in config file setting only. When IPv6 is set, the following restrict - version must be => 1.21 - setting vpc.NAT is not allowed - serviceIPv4CIDR is not supported together with IPv6 +- AutoAllocateIPv6 is not supported together with IPv6 The default value is `IPv4`. From 6ccf8a3583bc4f072bbaea1785382830a59b503c Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Wed, 6 Oct 2021 17:20:39 +0200 Subject: [PATCH 07/12] Unmanaged nodegroups are not supported --- pkg/actions/nodegroup/create.go | 5 +++ pkg/actions/nodegroup/create_test.go | 50 ++++++++++++++++++++++++++++ pkg/apis/eksctl.io/v1alpha5/types.go | 4 +-- pkg/utils/strings/strings.go | 8 +++++ userdocs/src/usage/vpc-networking.md | 1 + 5 files changed, 66 insertions(+), 2 deletions(-) diff --git a/pkg/actions/nodegroup/create.go b/pkg/actions/nodegroup/create.go index 57205cc409..5924d066bd 100644 --- a/pkg/actions/nodegroup/create.go +++ b/pkg/actions/nodegroup/create.go @@ -15,6 +15,7 @@ import ( "github.com/weaveworks/eksctl/pkg/kubernetes" "github.com/weaveworks/eksctl/pkg/printers" "github.com/weaveworks/eksctl/pkg/utils" + utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" "github.com/weaveworks/eksctl/pkg/utils/tasks" "github.com/weaveworks/eksctl/pkg/vpc" @@ -72,6 +73,10 @@ func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodegroupFil return errors.New("Managed Nodegroups are not supported for this cluster version. Please update the cluster before adding managed nodegroups") } + if utilsstrings.Value(cfg.VPC.IPFamily) == string(api.IPV6Family) && len(cfg.NodeGroups) > 0 { + return errors.New("unmanaged nodegroups are not supported with IPv6 clusters") + } + if err := eks.ValidateBottlerocketSupport(ctl.ControlPlaneVersion(), cmdutils.ToKubeNodeGroups(cfg)); err != nil { return err } diff --git a/pkg/actions/nodegroup/create_test.go b/pkg/actions/nodegroup/create_test.go index b85de28d7a..56208502f7 100644 --- a/pkg/actions/nodegroup/create_test.go +++ b/pkg/actions/nodegroup/create_test.go @@ -4,10 +4,12 @@ import ( "fmt" "strings" + . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" "github.com/pkg/errors" + "github.com/weaveworks/eksctl/pkg/actions/nodegroup" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/cfn/manager" @@ -17,6 +19,7 @@ import ( "github.com/weaveworks/eksctl/pkg/kubernetes" "github.com/weaveworks/eksctl/pkg/testutils" "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" + utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" ) type ngEntry struct { @@ -299,6 +302,53 @@ var _ = DescribeTable("Create", func(t ngEntry) { }), ) +var _ = Describe("create", func() { + When("creating an unmanaged nodegroup for an ipv6 cluster", func() { + It("returns an error", func() { + cfg := newClusterConfig() + cfg.Metadata.Version = api.Version1_21 + cfg.IAM = &api.ClusterIAM{ + WithOIDC: api.Enabled(), + } + cfg.Addons = []*api.Addon{ + { + Name: api.VPCCNIAddon, + }, + { + Name: api.KubeProxyAddon, + }, + { + Name: api.CoreDNSAddon, + }, + } + cfg.VPC.IPFamily = utilsstrings.Pointer(string(api.IPV6Family)) + + p := mockprovider.NewMockProvider() + ctl := &eks.ClusterProvider{ + Provider: p, + Status: &eks.ProviderStatus{ + ClusterInfo: &eks.ClusterInfo{ + Cluster: testutils.NewFakeCluster("my-cluster", ""), + }, + }, + } + m := nodegroup.New(cfg, ctl, nil) + + k := &fakes.FakeKubeProvider{} + k.SupportsManagedNodesReturns(true, nil) + m.MockKubeProvider(k) + + init := &fakes.FakeNodeGroupInitialiser{} + m.MockNodeGroupService(init) + + ngFilter := utilFakes.FakeNodegroupFilter{} + + err := m.Create(nodegroup.CreateOpts{}, &ngFilter) + Expect(err).To(MatchError(ContainSubstring("unmanaged nodegroups are not supported with IPv6 clusters"))) + }) + }) +}) + func newClusterConfig() *api.ClusterConfig { return &api.ClusterConfig{ TypeMeta: api.ClusterConfigTypeMeta(), diff --git a/pkg/apis/eksctl.io/v1alpha5/types.go b/pkg/apis/eksctl.io/v1alpha5/types.go index 30ac35f697..5292579cba 100644 --- a/pkg/apis/eksctl.io/v1alpha5/types.go +++ b/pkg/apis/eksctl.io/v1alpha5/types.go @@ -26,6 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" "github.com/weaveworks/eksctl/pkg/utils/taints" ) @@ -743,12 +744,11 @@ func NewClusterConfig() *ClusterConfig { // NewClusterVPC creates new VPC config for a cluster func NewClusterVPC() *ClusterVPC { cidr := DefaultCIDR() - defaultIPFamily := string(DefaultIPFamily) return &ClusterVPC{ Network: Network{ CIDR: &cidr, - IPFamily: &defaultIPFamily, + IPFamily: utilsstrings.Pointer(string(DefaultIPFamily)), }, ManageSharedNodeSecurityGroupRules: Enabled(), NAT: DefaultClusterNAT(), diff --git a/pkg/utils/strings/strings.go b/pkg/utils/strings/strings.go index ab45d8e9cd..1354bf82cb 100644 --- a/pkg/utils/strings/strings.go +++ b/pkg/utils/strings/strings.go @@ -7,6 +7,14 @@ func Pointer(s string) *string { return &s } +// Value returns the value of a pointer, empty string if it's nil. +func Value(s *string) string { + if s == nil { + return "" + } + return *s +} + // HasPrefix tests whether the string s begins with prefix. func HasPrefix(s, prefix string) bool { return strings.HasPrefix(s, prefix) diff --git a/userdocs/src/usage/vpc-networking.md b/userdocs/src/usage/vpc-networking.md index cea55a23a7..0c2ec5499b 100644 --- a/userdocs/src/usage/vpc-networking.md +++ b/userdocs/src/usage/vpc-networking.md @@ -59,6 +59,7 @@ This is an in config file setting only. When IPv6 is set, the following restrict - setting vpc.NAT is not allowed - serviceIPv4CIDR is not supported together with IPv6 - AutoAllocateIPv6 is not supported together with IPv6 +- unmanaged nodegroups are not yet supported with IPv6 clusters The default value is `IPv4`. From c52b1e0a4f6435a35af38c5d89dfcac716e9a8ac Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Thu, 7 Oct 2021 17:13:45 +0200 Subject: [PATCH 08/12] Nodegroup creation is not supported with unowned ipv6 clusters --- pkg/actions/nodegroup/create.go | 9 ++- pkg/actions/nodegroup/create_test.go | 115 ++++++++++++++++++++++++++- userdocs/src/usage/vpc-networking.md | 1 + 3 files changed, 122 insertions(+), 3 deletions(-) diff --git a/pkg/actions/nodegroup/create.go b/pkg/actions/nodegroup/create.go index 5924d066bd..2e1b4cb9fb 100644 --- a/pkg/actions/nodegroup/create.go +++ b/pkg/actions/nodegroup/create.go @@ -73,8 +73,13 @@ func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodegroupFil return errors.New("Managed Nodegroups are not supported for this cluster version. Please update the cluster before adding managed nodegroups") } - if utilsstrings.Value(cfg.VPC.IPFamily) == string(api.IPV6Family) && len(cfg.NodeGroups) > 0 { - return errors.New("unmanaged nodegroups are not supported with IPv6 clusters") + if utilsstrings.Value(cfg.VPC.IPFamily) == string(api.IPV6Family) { + if len(cfg.NodeGroups) > 0 { + return errors.New("unmanaged nodegroups are not supported with IPv6 clusters") + } + if !isOwnedCluster && len(cfg.ManagedNodeGroups) > 0 { + return errors.New("nodegroups cannot be created on IPv6 unowned clusters") + } } if err := eks.ValidateBottlerocketSupport(ctl.ControlPlaneVersion(), cmdutils.ToKubeNodeGroups(cfg)); err != nil { diff --git a/pkg/actions/nodegroup/create_test.go b/pkg/actions/nodegroup/create_test.go index 56208502f7..c853929dc2 100644 --- a/pkg/actions/nodegroup/create_test.go +++ b/pkg/actions/nodegroup/create_test.go @@ -4,11 +4,13 @@ import ( "fmt" "strings" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" - "github.com/pkg/errors" + "github.com/stretchr/testify/mock" "github.com/weaveworks/eksctl/pkg/actions/nodegroup" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" @@ -347,6 +349,117 @@ var _ = Describe("create", func() { Expect(err).To(MatchError(ContainSubstring("unmanaged nodegroups are not supported with IPv6 clusters"))) }) }) + When("creating a nodegroups for unowned ipv6 cluster", func() { + It("returns an error", func() { + cfg := newClusterConfig() + cfg.Metadata.Version = api.Version1_21 + cfg.IAM = &api.ClusterIAM{ + WithOIDC: api.Enabled(), + } + cfg.Addons = []*api.Addon{ + { + Name: api.VPCCNIAddon, + }, + { + Name: api.KubeProxyAddon, + }, + { + Name: api.CoreDNSAddon, + }, + } + cfg.VPC.IPFamily = utilsstrings.Pointer(string(api.IPV6Family)) + + p := mockprovider.NewMockProvider() + p.MockEC2().On("DescribeVpcs", &ec2.DescribeVpcsInput{VpcIds: aws.StringSlice([]string{"custom"})}).Return(&ec2.DescribeVpcsOutput{ + Vpcs: []*ec2.Vpc{ + { + VpcId: aws.String("custom"), + CidrBlock: aws.String("192.168.0.0/16"), + }, + }, + }, nil) + p.MockEC2().On("DescribeSubnets", &ec2.DescribeSubnetsInput{ + SubnetIds: aws.StringSlice([]string{"sn-private-1", "sn-private-2"}), + }).Return(&ec2.DescribeSubnetsOutput{ + Subnets: []*ec2.Subnet{ + { + SubnetId: aws.String("sn-private-1"), + VpcId: aws.String("custom"), + AvailabilityZone: aws.String("us-east-1b"), + CidrBlock: aws.String("192.168.0.0/24"), + }, + { + SubnetId: aws.String("sn-private-2"), + VpcId: aws.String("custom"), + AvailabilityZone: aws.String("us-east-1b"), + CidrBlock: aws.String("192.168.0.1/24"), + }, + }, + }, nil) + // The order is undetermined because the entries are maps and we don't sort the ids. + p.MockEC2().On("DescribeSubnets", mock.Anything).Return(&ec2.DescribeSubnetsOutput{ + Subnets: []*ec2.Subnet{ + { + SubnetId: aws.String("sn-public-1"), + VpcId: aws.String("custom"), + AvailabilityZone: aws.String("us-east-1b"), + CidrBlock: aws.String("192.168.1.0/24"), + }, + { + SubnetId: aws.String("sn-public-2"), + VpcId: aws.String("custom"), + AvailabilityZone: aws.String("us-east-1b"), + CidrBlock: aws.String("192.168.2.0/24"), + }, + { + SubnetId: aws.String("sn-public-3"), + VpcId: aws.String("custom"), + AvailabilityZone: aws.String("us-east-1b"), + CidrBlock: aws.String("192.168.3.0/24"), + }, + { + SubnetId: aws.String("sn-public-4"), + VpcId: aws.String("custom"), + AvailabilityZone: aws.String("us-east-1b"), + CidrBlock: aws.String("192.168.4.0/24"), + }, + }, + }, nil) + ctl := &eks.ClusterProvider{ + Provider: p, + Status: &eks.ProviderStatus{ + ClusterInfo: &eks.ClusterInfo{ + Cluster: testutils.NewFakeCluster("my-cluster", ""), + }, + }, + } + cfg.VPC.Subnets = &api.ClusterSubnets{ + Public: api.AZSubnetMapping{ + "sn-public-1": api.AZSubnetSpec{ID: "sn-public-1"}, + "sn-public-2": api.AZSubnetSpec{ID: "sn-public-2"}, + "sn-public-3": api.AZSubnetSpec{ID: "sn-public-3"}, + "sn-public-4": api.AZSubnetSpec{ID: "sn-public-4"}, + }, + } + cfg.VPC.ID = "custom" + cfg.VPC.SecurityGroup = "sg" + cfg.NodeGroups = nil + m := nodegroup.New(cfg, ctl, nil) + + k := &fakes.FakeKubeProvider{} + k.LoadClusterIntoSpecFromStackReturns(&manager.StackNotFoundErr{}) + k.SupportsManagedNodesReturns(true, nil) + m.MockKubeProvider(k) + + init := &fakes.FakeNodeGroupInitialiser{} + m.MockNodeGroupService(init) + + ngFilter := utilFakes.FakeNodegroupFilter{} + + err := m.Create(nodegroup.CreateOpts{}, &ngFilter) + Expect(err).To(MatchError(ContainSubstring("nodegroups cannot be created on IPv6 unowned clusters"))) + }) + }) }) func newClusterConfig() *api.ClusterConfig { diff --git a/userdocs/src/usage/vpc-networking.md b/userdocs/src/usage/vpc-networking.md index 0c2ec5499b..b70fc7d417 100644 --- a/userdocs/src/usage/vpc-networking.md +++ b/userdocs/src/usage/vpc-networking.md @@ -60,6 +60,7 @@ This is an in config file setting only. When IPv6 is set, the following restrict - serviceIPv4CIDR is not supported together with IPv6 - AutoAllocateIPv6 is not supported together with IPv6 - unmanaged nodegroups are not yet supported with IPv6 clusters +- nodegroup creation is not supported with un-owned IPv6 clusters The default value is `IPv4`. From 14341877ed45d799f58edfd853a09e2b6cdfee41 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Fri, 15 Oct 2021 09:40:38 +0200 Subject: [PATCH 09/12] First iteration of modifying the interface for ownership determination --- pkg/actions/nodegroup/create.go | 16 +- pkg/actions/nodegroup/create_test.go | 163 ------------------ .../builder/managed_launch_template_test.go | 8 +- pkg/cfn/builder/managed_nodegroup.go | 8 +- pkg/cfn/builder/managed_nodegroup_test.go | 26 ++- pkg/cfn/builder/nodegroup.go | 1 - pkg/cfn/manager/api.go | 1 + pkg/cfn/manager/create_tasks.go | 8 +- pkg/cfn/manager/fakes/fake_stack_manager.go | 18 +- pkg/cfn/manager/interface.go | 3 +- pkg/cfn/manager/nodegroup.go | 5 +- pkg/cfn/manager/tasks.go | 3 +- pkg/eks/mocks/KubeNodeGroup.go | 2 +- 13 files changed, 60 insertions(+), 202 deletions(-) diff --git a/pkg/actions/nodegroup/create.go b/pkg/actions/nodegroup/create.go index 2e1b4cb9fb..c917161af6 100644 --- a/pkg/actions/nodegroup/create.go +++ b/pkg/actions/nodegroup/create.go @@ -15,7 +15,6 @@ import ( "github.com/weaveworks/eksctl/pkg/kubernetes" "github.com/weaveworks/eksctl/pkg/printers" "github.com/weaveworks/eksctl/pkg/utils" - utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" "github.com/weaveworks/eksctl/pkg/utils/tasks" "github.com/weaveworks/eksctl/pkg/vpc" @@ -73,15 +72,6 @@ func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodegroupFil return errors.New("Managed Nodegroups are not supported for this cluster version. Please update the cluster before adding managed nodegroups") } - if utilsstrings.Value(cfg.VPC.IPFamily) == string(api.IPV6Family) { - if len(cfg.NodeGroups) > 0 { - return errors.New("unmanaged nodegroups are not supported with IPv6 clusters") - } - if !isOwnedCluster && len(cfg.ManagedNodeGroups) > 0 { - return errors.New("nodegroups cannot be created on IPv6 unowned clusters") - } - } - if err := eks.ValidateBottlerocketSupport(ctl.ControlPlaneVersion(), cmdutils.ToKubeNodeGroups(cfg)); err != nil { return err } @@ -142,7 +132,7 @@ func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodegroupFil return cmdutils.PrintNodeGroupDryRunConfig(clusterConfigCopy, os.Stdout) } - if err := m.nodeCreationTasks(options, nodegroupFilter, supportsManagedNodes, isOwnedCluster); err != nil { + if err := m.nodeCreationTasks(supportsManagedNodes, isOwnedCluster); err != nil { return err } @@ -157,7 +147,7 @@ func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodegroupFil return nil } -func (m *Manager) nodeCreationTasks(options CreateOpts, nodegroupFilter filter.NodegroupFilter, supportsManagedNodes, isOwnedCluster bool) error { +func (m *Manager) nodeCreationTasks(supportsManagedNodes, isOwnedCluster bool) error { cfg := m.cfg meta := cfg.Metadata init := m.init @@ -193,7 +183,7 @@ func (m *Manager) nodeCreationTasks(options CreateOpts, nodegroupFilter filter.N if nodeGroupTasks.Len() > 0 { allNodeGroupTasks.Append(nodeGroupTasks) } - managedTasks := m.stackManager.NewManagedNodeGroupTask(cfg.ManagedNodeGroups, !awsNodeUsesIRSA, vpcImporter) + managedTasks := m.stackManager.NewManagedNodeGroupTask(cfg.ManagedNodeGroups, !awsNodeUsesIRSA, vpcImporter, isOwnedCluster) if managedTasks.Len() > 0 { allNodeGroupTasks.Append(managedTasks) } diff --git a/pkg/actions/nodegroup/create_test.go b/pkg/actions/nodegroup/create_test.go index c853929dc2..d2f3246f8b 100644 --- a/pkg/actions/nodegroup/create_test.go +++ b/pkg/actions/nodegroup/create_test.go @@ -4,13 +4,9 @@ import ( "fmt" "strings" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ec2" - . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" "github.com/pkg/errors" - "github.com/stretchr/testify/mock" "github.com/weaveworks/eksctl/pkg/actions/nodegroup" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" @@ -21,7 +17,6 @@ import ( "github.com/weaveworks/eksctl/pkg/kubernetes" "github.com/weaveworks/eksctl/pkg/testutils" "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" - utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" ) type ngEntry struct { @@ -304,164 +299,6 @@ var _ = DescribeTable("Create", func(t ngEntry) { }), ) -var _ = Describe("create", func() { - When("creating an unmanaged nodegroup for an ipv6 cluster", func() { - It("returns an error", func() { - cfg := newClusterConfig() - cfg.Metadata.Version = api.Version1_21 - cfg.IAM = &api.ClusterIAM{ - WithOIDC: api.Enabled(), - } - cfg.Addons = []*api.Addon{ - { - Name: api.VPCCNIAddon, - }, - { - Name: api.KubeProxyAddon, - }, - { - Name: api.CoreDNSAddon, - }, - } - cfg.VPC.IPFamily = utilsstrings.Pointer(string(api.IPV6Family)) - - p := mockprovider.NewMockProvider() - ctl := &eks.ClusterProvider{ - Provider: p, - Status: &eks.ProviderStatus{ - ClusterInfo: &eks.ClusterInfo{ - Cluster: testutils.NewFakeCluster("my-cluster", ""), - }, - }, - } - m := nodegroup.New(cfg, ctl, nil) - - k := &fakes.FakeKubeProvider{} - k.SupportsManagedNodesReturns(true, nil) - m.MockKubeProvider(k) - - init := &fakes.FakeNodeGroupInitialiser{} - m.MockNodeGroupService(init) - - ngFilter := utilFakes.FakeNodegroupFilter{} - - err := m.Create(nodegroup.CreateOpts{}, &ngFilter) - Expect(err).To(MatchError(ContainSubstring("unmanaged nodegroups are not supported with IPv6 clusters"))) - }) - }) - When("creating a nodegroups for unowned ipv6 cluster", func() { - It("returns an error", func() { - cfg := newClusterConfig() - cfg.Metadata.Version = api.Version1_21 - cfg.IAM = &api.ClusterIAM{ - WithOIDC: api.Enabled(), - } - cfg.Addons = []*api.Addon{ - { - Name: api.VPCCNIAddon, - }, - { - Name: api.KubeProxyAddon, - }, - { - Name: api.CoreDNSAddon, - }, - } - cfg.VPC.IPFamily = utilsstrings.Pointer(string(api.IPV6Family)) - - p := mockprovider.NewMockProvider() - p.MockEC2().On("DescribeVpcs", &ec2.DescribeVpcsInput{VpcIds: aws.StringSlice([]string{"custom"})}).Return(&ec2.DescribeVpcsOutput{ - Vpcs: []*ec2.Vpc{ - { - VpcId: aws.String("custom"), - CidrBlock: aws.String("192.168.0.0/16"), - }, - }, - }, nil) - p.MockEC2().On("DescribeSubnets", &ec2.DescribeSubnetsInput{ - SubnetIds: aws.StringSlice([]string{"sn-private-1", "sn-private-2"}), - }).Return(&ec2.DescribeSubnetsOutput{ - Subnets: []*ec2.Subnet{ - { - SubnetId: aws.String("sn-private-1"), - VpcId: aws.String("custom"), - AvailabilityZone: aws.String("us-east-1b"), - CidrBlock: aws.String("192.168.0.0/24"), - }, - { - SubnetId: aws.String("sn-private-2"), - VpcId: aws.String("custom"), - AvailabilityZone: aws.String("us-east-1b"), - CidrBlock: aws.String("192.168.0.1/24"), - }, - }, - }, nil) - // The order is undetermined because the entries are maps and we don't sort the ids. - p.MockEC2().On("DescribeSubnets", mock.Anything).Return(&ec2.DescribeSubnetsOutput{ - Subnets: []*ec2.Subnet{ - { - SubnetId: aws.String("sn-public-1"), - VpcId: aws.String("custom"), - AvailabilityZone: aws.String("us-east-1b"), - CidrBlock: aws.String("192.168.1.0/24"), - }, - { - SubnetId: aws.String("sn-public-2"), - VpcId: aws.String("custom"), - AvailabilityZone: aws.String("us-east-1b"), - CidrBlock: aws.String("192.168.2.0/24"), - }, - { - SubnetId: aws.String("sn-public-3"), - VpcId: aws.String("custom"), - AvailabilityZone: aws.String("us-east-1b"), - CidrBlock: aws.String("192.168.3.0/24"), - }, - { - SubnetId: aws.String("sn-public-4"), - VpcId: aws.String("custom"), - AvailabilityZone: aws.String("us-east-1b"), - CidrBlock: aws.String("192.168.4.0/24"), - }, - }, - }, nil) - ctl := &eks.ClusterProvider{ - Provider: p, - Status: &eks.ProviderStatus{ - ClusterInfo: &eks.ClusterInfo{ - Cluster: testutils.NewFakeCluster("my-cluster", ""), - }, - }, - } - cfg.VPC.Subnets = &api.ClusterSubnets{ - Public: api.AZSubnetMapping{ - "sn-public-1": api.AZSubnetSpec{ID: "sn-public-1"}, - "sn-public-2": api.AZSubnetSpec{ID: "sn-public-2"}, - "sn-public-3": api.AZSubnetSpec{ID: "sn-public-3"}, - "sn-public-4": api.AZSubnetSpec{ID: "sn-public-4"}, - }, - } - cfg.VPC.ID = "custom" - cfg.VPC.SecurityGroup = "sg" - cfg.NodeGroups = nil - m := nodegroup.New(cfg, ctl, nil) - - k := &fakes.FakeKubeProvider{} - k.LoadClusterIntoSpecFromStackReturns(&manager.StackNotFoundErr{}) - k.SupportsManagedNodesReturns(true, nil) - m.MockKubeProvider(k) - - init := &fakes.FakeNodeGroupInitialiser{} - m.MockNodeGroupService(init) - - ngFilter := utilFakes.FakeNodegroupFilter{} - - err := m.Create(nodegroup.CreateOpts{}, &ngFilter) - Expect(err).To(MatchError(ContainSubstring("nodegroups cannot be created on IPv6 unowned clusters"))) - }) - }) -}) - func newClusterConfig() *api.ClusterConfig { return &api.ClusterConfig{ TypeMeta: api.ClusterConfigTypeMeta(), diff --git a/pkg/cfn/builder/managed_launch_template_test.go b/pkg/cfn/builder/managed_launch_template_test.go index 8be5098e4f..76a4b6ff43 100644 --- a/pkg/cfn/builder/managed_launch_template_test.go +++ b/pkg/cfn/builder/managed_launch_template_test.go @@ -13,14 +13,16 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" + "github.com/weaveworks/eksctl/pkg/nodebootstrap/fakes" "github.com/stretchr/testify/mock" + "github.com/weaveworks/goformation/v4" + gfnt "github.com/weaveworks/goformation/v4/cloudformation/types" + api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" vpcfakes "github.com/weaveworks/eksctl/pkg/vpc/fakes" - "github.com/weaveworks/goformation/v4" - gfnt "github.com/weaveworks/goformation/v4/cloudformation/types" ) type mngCase struct { @@ -58,7 +60,7 @@ var _ = Describe("ManagedNodeGroup builder", func() { return userData, nil } - stack := NewManagedNodeGroup(provider.MockEC2(), clusterConfig, m.ng, NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, fakeVPCImporter) + stack := NewManagedNodeGroup(provider.MockEC2(), clusterConfig, m.ng, NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, fakeVPCImporter, true) err := stack.AddAllResources() if m.errMsg != "" { Expect(err).To(HaveOccurred()) diff --git a/pkg/cfn/builder/managed_nodegroup.go b/pkg/cfn/builder/managed_nodegroup.go index fe50c7f915..5ed3083e12 100644 --- a/pkg/cfn/builder/managed_nodegroup.go +++ b/pkg/cfn/builder/managed_nodegroup.go @@ -15,6 +15,7 @@ import ( api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/nodebootstrap" "github.com/weaveworks/eksctl/pkg/utils" + utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" "github.com/weaveworks/eksctl/pkg/vpc" ) @@ -27,13 +28,14 @@ type ManagedNodeGroupResourceSet struct { ec2API ec2iface.EC2API vpcImporter vpc.Importer bootstrapper nodebootstrap.Bootstrapper + isOwnedCluster bool *resourceSet } const ManagedNodeGroupResourceName = "ManagedNodeGroup" // NewManagedNodeGroup creates a new ManagedNodeGroupResourceSet -func NewManagedNodeGroup(ec2API ec2iface.EC2API, cluster *api.ClusterConfig, nodeGroup *api.ManagedNodeGroup, launchTemplateFetcher *LaunchTemplateFetcher, bootstrapper nodebootstrap.Bootstrapper, forceAddCNIPolicy bool, vpcImporter vpc.Importer) *ManagedNodeGroupResourceSet { +func NewManagedNodeGroup(ec2API ec2iface.EC2API, cluster *api.ClusterConfig, nodeGroup *api.ManagedNodeGroup, launchTemplateFetcher *LaunchTemplateFetcher, bootstrapper nodebootstrap.Bootstrapper, forceAddCNIPolicy bool, vpcImporter vpc.Importer, isOwnedCluster bool) *ManagedNodeGroupResourceSet { return &ManagedNodeGroupResourceSet{ clusterConfig: cluster, forceAddCNIPolicy: forceAddCNIPolicy, @@ -43,11 +45,15 @@ func NewManagedNodeGroup(ec2API ec2iface.EC2API, cluster *api.ClusterConfig, nod resourceSet: newResourceSet(), vpcImporter: vpcImporter, bootstrapper: bootstrapper, + isOwnedCluster: isOwnedCluster, } } // AddAllResources adds all required CloudFormation resources func (m *ManagedNodeGroupResourceSet) AddAllResources() error { + if utilsstrings.Value(m.clusterConfig.VPC.IPFamily) == string(api.IPV6Family) && !m.isOwnedCluster { + return errors.New("managed nodegroups cannot be created on IPv6 unowned clusters") + } m.resourceSet.template.Description = fmt.Sprintf( "%s (SSH access: %v) %s", "EKS Managed Nodes", diff --git a/pkg/cfn/builder/managed_nodegroup_test.go b/pkg/cfn/builder/managed_nodegroup_test.go index 03cd7c3bb1..afe85d4663 100644 --- a/pkg/cfn/builder/managed_nodegroup_test.go +++ b/pkg/cfn/builder/managed_nodegroup_test.go @@ -5,14 +5,16 @@ import ( "testing" "github.com/stretchr/testify/require" + "github.com/weaveworks/goformation/v4" + gfneks "github.com/weaveworks/goformation/v4/cloudformation/eks" + gfnt "github.com/weaveworks/goformation/v4/cloudformation/types" + api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/nodebootstrap" "github.com/weaveworks/eksctl/pkg/nodebootstrap/fakes" "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" + utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" vpcfakes "github.com/weaveworks/eksctl/pkg/vpc/fakes" - "github.com/weaveworks/goformation/v4" - gfneks "github.com/weaveworks/goformation/v4/cloudformation/eks" - gfnt "github.com/weaveworks/goformation/v4/cloudformation/types" ) func TestManagedPolicyResources(t *testing.T) { @@ -89,7 +91,7 @@ func TestManagedPolicyResources(t *testing.T) { bootstrapper.UserDataStub = func() (string, error) { return "", nil } - stack := NewManagedNodeGroup(p.EC2(), clusterConfig, ng, nil, bootstrapper, false, fakeVPCImporter) + stack := NewManagedNodeGroup(p.EC2(), clusterConfig, ng, nil, bootstrapper, false, fakeVPCImporter, true) err := stack.AddAllResources() require.Nil(err) @@ -158,7 +160,7 @@ func TestManagedNodeRole(t *testing.T) { p := mockprovider.NewMockProvider() fakeVPCImporter := new(vpcfakes.FakeImporter) bootstrapper := nodebootstrap.NewManagedBootstrapper(clusterConfig, tt.nodeGroup) - stack := NewManagedNodeGroup(p.EC2(), clusterConfig, tt.nodeGroup, nil, bootstrapper, false, fakeVPCImporter) + stack := NewManagedNodeGroup(p.EC2(), clusterConfig, tt.nodeGroup, nil, bootstrapper, false, fakeVPCImporter, true) err := stack.AddAllResources() require.NoError(err) @@ -179,6 +181,20 @@ func TestManagedNodeRole(t *testing.T) { } } +func TestManagedNodeGroupAddAllResources_WithUnownedIPv6Cluster(t *testing.T) { + ng := &api.ManagedNodeGroup{ + NodeGroupBase: &api.NodeGroupBase{}, + } + p := mockprovider.NewMockProvider() + clusterConfig := api.NewClusterConfig() + clusterConfig.VPC.IPFamily = utilsstrings.Pointer(string(api.IPV6Family)) + fakeVPCImporter := new(vpcfakes.FakeImporter) + bootstrapper := nodebootstrap.NewManagedBootstrapper(clusterConfig, ng) + stack := NewManagedNodeGroup(p.EC2(), clusterConfig, ng, nil, bootstrapper, false, fakeVPCImporter, false) + err := stack.AddAllResources() + require.EqualError(t, err, "managed nodegroups cannot be created on IPv6 unowned clusters") +} + func makePartitionedPolicies(policies ...string) []*gfnt.Value { var partitionedPolicies []*gfnt.Value for _, policy := range policies { diff --git a/pkg/cfn/builder/nodegroup.go b/pkg/cfn/builder/nodegroup.go index ce14d7d651..e5f9dad7b2 100644 --- a/pkg/cfn/builder/nodegroup.go +++ b/pkg/cfn/builder/nodegroup.go @@ -52,7 +52,6 @@ func NewNodeGroupResourceSet(ec2API ec2iface.EC2API, iamAPI iamiface.IAMAPI, spe // AddAllResources adds all the information about the nodegroup to the resource set func (n *NodeGroupResourceSet) AddAllResources() error { - if utilsstrings.Value(n.clusterSpec.VPC.IPFamily) == string(api.IPV6Family) { return errors.New("unmanaged nodegroups are not supported with IPv6 clusters") } diff --git a/pkg/cfn/manager/api.go b/pkg/cfn/manager/api.go index ae177e89eb..53bbb4923e 100644 --- a/pkg/cfn/manager/api.go +++ b/pkg/cfn/manager/api.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface" "github.com/aws/aws-sdk-go/service/cloudtrail/cloudtrailiface" + "github.com/weaveworks/eksctl/pkg/cfn/waiter" "github.com/aws/aws-sdk-go/service/cloudformation/cloudformationiface" diff --git a/pkg/cfn/manager/create_tasks.go b/pkg/cfn/manager/create_tasks.go index a038e31aea..d54762533e 100644 --- a/pkg/cfn/manager/create_tasks.go +++ b/pkg/cfn/manager/create_tasks.go @@ -5,12 +5,13 @@ import ( "github.com/kris-nova/logger" "github.com/pkg/errors" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" iamoidc "github.com/weaveworks/eksctl/pkg/iam/oidc" "github.com/weaveworks/eksctl/pkg/kubernetes" "github.com/weaveworks/eksctl/pkg/utils/tasks" "github.com/weaveworks/eksctl/pkg/vpc" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -36,7 +37,7 @@ func (c *StackCollection) NewTasksToCreateClusterWithNodeGroups(nodeGroups []*ap appendNodeGroupTasksTo := func(taskTree *tasks.TaskTree) { vpcImporter := vpc.NewStackConfigImporter(c.MakeClusterStackName()) nodeGroupTasks := c.NewUnmanagedNodeGroupTask(nodeGroups, false, vpcImporter) - managedNodeGroupTasks := c.NewManagedNodeGroupTask(managedNodeGroups, false, vpcImporter) + managedNodeGroupTasks := c.NewManagedNodeGroupTask(managedNodeGroups, false, vpcImporter, true) if managedNodeGroupTasks.Len() > 0 { nodeGroupTasks.Append(managedNodeGroupTasks.Tasks...) } @@ -81,7 +82,7 @@ func (c *StackCollection) NewUnmanagedNodeGroupTask(nodeGroups []*api.NodeGroup, } // NewManagedNodeGroupTask defines tasks required to create managed nodegroups -func (c *StackCollection) NewManagedNodeGroupTask(nodeGroups []*api.ManagedNodeGroup, forceAddCNIPolicy bool, vpcImporter vpc.Importer) *tasks.TaskTree { +func (c *StackCollection) NewManagedNodeGroupTask(nodeGroups []*api.ManagedNodeGroup, forceAddCNIPolicy bool, vpcImporter vpc.Importer, isOwnedCluster bool) *tasks.TaskTree { taskTree := &tasks.TaskTree{Parallel: true} for _, ng := range nodeGroups { taskTree.Append(&managedNodeGroupTask{ @@ -89,6 +90,7 @@ func (c *StackCollection) NewManagedNodeGroupTask(nodeGroups []*api.ManagedNodeG nodeGroup: ng, forceAddCNIPolicy: forceAddCNIPolicy, vpcImporter: vpcImporter, + isOwnedCluster: isOwnedCluster, info: fmt.Sprintf("create managed nodegroup %q", ng.Name), }) } diff --git a/pkg/cfn/manager/fakes/fake_stack_manager.go b/pkg/cfn/manager/fakes/fake_stack_manager.go index 4a5b102d9c..f4b44d9145 100644 --- a/pkg/cfn/manager/fakes/fake_stack_manager.go +++ b/pkg/cfn/manager/fakes/fake_stack_manager.go @@ -522,12 +522,13 @@ type FakeStackManager struct { newClusterCompatTaskReturnsOnCall map[int]struct { result1 tasks.Task } - NewManagedNodeGroupTaskStub func([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer) *tasks.TaskTree + NewManagedNodeGroupTaskStub func([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer, bool) *tasks.TaskTree newManagedNodeGroupTaskMutex sync.RWMutex newManagedNodeGroupTaskArgsForCall []struct { arg1 []*v1alpha5.ManagedNodeGroup arg2 bool arg3 vpc.Importer + arg4 bool } newManagedNodeGroupTaskReturns struct { result1 *tasks.TaskTree @@ -3196,7 +3197,7 @@ func (fake *FakeStackManager) NewClusterCompatTaskReturnsOnCall(i int, result1 t }{result1} } -func (fake *FakeStackManager) NewManagedNodeGroupTask(arg1 []*v1alpha5.ManagedNodeGroup, arg2 bool, arg3 vpc.Importer) *tasks.TaskTree { +func (fake *FakeStackManager) NewManagedNodeGroupTask(arg1 []*v1alpha5.ManagedNodeGroup, arg2 bool, arg3 vpc.Importer, arg4 bool) *tasks.TaskTree { var arg1Copy []*v1alpha5.ManagedNodeGroup if arg1 != nil { arg1Copy = make([]*v1alpha5.ManagedNodeGroup, len(arg1)) @@ -3208,13 +3209,14 @@ func (fake *FakeStackManager) NewManagedNodeGroupTask(arg1 []*v1alpha5.ManagedNo arg1 []*v1alpha5.ManagedNodeGroup arg2 bool arg3 vpc.Importer - }{arg1Copy, arg2, arg3}) + arg4 bool + }{arg1Copy, arg2, arg3, arg4}) stub := fake.NewManagedNodeGroupTaskStub fakeReturns := fake.newManagedNodeGroupTaskReturns - fake.recordInvocation("NewManagedNodeGroupTask", []interface{}{arg1Copy, arg2, arg3}) + fake.recordInvocation("NewManagedNodeGroupTask", []interface{}{arg1Copy, arg2, arg3, arg4}) fake.newManagedNodeGroupTaskMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3) + return stub(arg1, arg2, arg3, arg4) } if specificReturn { return ret.result1 @@ -3228,17 +3230,17 @@ func (fake *FakeStackManager) NewManagedNodeGroupTaskCallCount() int { return len(fake.newManagedNodeGroupTaskArgsForCall) } -func (fake *FakeStackManager) NewManagedNodeGroupTaskCalls(stub func([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer) *tasks.TaskTree) { +func (fake *FakeStackManager) NewManagedNodeGroupTaskCalls(stub func([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer, bool) *tasks.TaskTree) { fake.newManagedNodeGroupTaskMutex.Lock() defer fake.newManagedNodeGroupTaskMutex.Unlock() fake.NewManagedNodeGroupTaskStub = stub } -func (fake *FakeStackManager) NewManagedNodeGroupTaskArgsForCall(i int) ([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer) { +func (fake *FakeStackManager) NewManagedNodeGroupTaskArgsForCall(i int) ([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer, bool) { fake.newManagedNodeGroupTaskMutex.RLock() defer fake.newManagedNodeGroupTaskMutex.RUnlock() argsForCall := fake.newManagedNodeGroupTaskArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 } func (fake *FakeStackManager) NewManagedNodeGroupTaskReturns(result1 *tasks.TaskTree) { diff --git a/pkg/cfn/manager/interface.go b/pkg/cfn/manager/interface.go index 4bdf4b9c17..5bb3565fc0 100644 --- a/pkg/cfn/manager/interface.go +++ b/pkg/cfn/manager/interface.go @@ -4,6 +4,7 @@ import ( "github.com/aws/aws-sdk-go/service/cloudformation" "github.com/aws/aws-sdk-go/service/cloudtrail" "github.com/aws/aws-sdk-go/service/eks/eksiface" + "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/cfn/builder" iamoidc "github.com/weaveworks/eksctl/pkg/iam/oidc" @@ -54,7 +55,7 @@ type StackManager interface { NewTasksToCreateClusterWithNodeGroups(nodeGroups []*v1alpha5.NodeGroup, managedNodeGroups []*v1alpha5.ManagedNodeGroup, supportsManagedNodes bool, postClusterCreationTasks ...tasks.Task) *tasks.TaskTree NewUnmanagedNodeGroupTask(nodeGroups []*v1alpha5.NodeGroup, forceAddCNIPolicy bool, importer vpc.Importer) *tasks.TaskTree - NewManagedNodeGroupTask(nodeGroups []*v1alpha5.ManagedNodeGroup, forceAddCNIPolicy bool, importer vpc.Importer) *tasks.TaskTree + NewManagedNodeGroupTask(nodeGroups []*v1alpha5.ManagedNodeGroup, forceAddCNIPolicy bool, importer vpc.Importer, isOwnedCluster bool) *tasks.TaskTree NewClusterCompatTask() tasks.Task NewTasksToCreateIAMServiceAccounts(serviceAccounts []*v1alpha5.ClusterIAMServiceAccount, oidc *iamoidc.OpenIDConnectManager, clientSetGetter kubernetes.ClientSetGetter) *tasks.TaskTree DeleteTasksForDeprecatedStacks() (*tasks.TaskTree, error) diff --git a/pkg/cfn/manager/nodegroup.go b/pkg/cfn/manager/nodegroup.go index a9724a2798..5d88d5f565 100644 --- a/pkg/cfn/manager/nodegroup.go +++ b/pkg/cfn/manager/nodegroup.go @@ -13,6 +13,7 @@ import ( "github.com/kris-nova/logger" "github.com/pkg/errors" "github.com/tidwall/gjson" + "github.com/weaveworks/eksctl/pkg/nodebootstrap" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" @@ -78,12 +79,12 @@ func (c *StackCollection) createNodeGroupTask(errs chan error, ng *api.NodeGroup return c.CreateStack(name, stack, ng.Tags, nil, errs) } -func (c *StackCollection) createManagedNodeGroupTask(errorCh chan error, ng *api.ManagedNodeGroup, forceAddCNIPolicy bool, vpcImporter vpc.Importer) error { +func (c *StackCollection) createManagedNodeGroupTask(errorCh chan error, ng *api.ManagedNodeGroup, forceAddCNIPolicy bool, vpcImporter vpc.Importer, isOwnedCluster bool) error { name := c.makeNodeGroupStackName(ng.Name) logger.Info("building managed nodegroup stack %q", name) bootstrapper := nodebootstrap.NewManagedBootstrapper(c.spec, ng) - stack := builder.NewManagedNodeGroup(c.ec2API, c.spec, ng, builder.NewLaunchTemplateFetcher(c.ec2API), bootstrapper, forceAddCNIPolicy, vpcImporter) + stack := builder.NewManagedNodeGroup(c.ec2API, c.spec, ng, builder.NewLaunchTemplateFetcher(c.ec2API), bootstrapper, forceAddCNIPolicy, vpcImporter, isOwnedCluster) if err := stack.AddAllResources(); err != nil { return err } diff --git a/pkg/cfn/manager/tasks.go b/pkg/cfn/manager/tasks.go index a4f1a36c03..b89318a4cf 100644 --- a/pkg/cfn/manager/tasks.go +++ b/pkg/cfn/manager/tasks.go @@ -43,12 +43,13 @@ type managedNodeGroupTask struct { stackCollection *StackCollection forceAddCNIPolicy bool vpcImporter vpc.Importer + isOwnedCluster bool } func (t *managedNodeGroupTask) Describe() string { return t.info } func (t *managedNodeGroupTask) Do(errorCh chan error) error { - return t.stackCollection.createManagedNodeGroupTask(errorCh, t.nodeGroup, t.forceAddCNIPolicy, t.vpcImporter) + return t.stackCollection.createManagedNodeGroupTask(errorCh, t.nodeGroup, t.forceAddCNIPolicy, t.vpcImporter, t.isOwnedCluster) } type clusterCompatTask struct { diff --git a/pkg/eks/mocks/KubeNodeGroup.go b/pkg/eks/mocks/KubeNodeGroup.go index 49e2ea4f2b..c4714e2b50 100644 --- a/pkg/eks/mocks/KubeNodeGroup.go +++ b/pkg/eks/mocks/KubeNodeGroup.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0. DO NOT EDIT. +// Code generated by mockery v0.0.0-dev. DO NOT EDIT. package mocks From d7a91da1ccb3cf86bc26227dce997fb700fa664a Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Fri, 15 Oct 2021 10:06:22 +0200 Subject: [PATCH 10/12] Changed but kept the check in nodegroup builder --- pkg/actions/nodegroup/create.go | 2 +- pkg/cfn/manager/api.go | 4 ++-- pkg/cfn/manager/create_tasks.go | 5 ++--- pkg/cfn/manager/fakes/fake_stack_manager.go | 18 ++++++++---------- pkg/cfn/manager/interface.go | 2 +- pkg/cfn/manager/nodegroup.go | 8 ++++++-- pkg/cfn/manager/tasks.go | 3 +-- pkg/eks/mocks/KubeNodeGroup.go | 2 +- 8 files changed, 22 insertions(+), 22 deletions(-) diff --git a/pkg/actions/nodegroup/create.go b/pkg/actions/nodegroup/create.go index c917161af6..1c3e385783 100644 --- a/pkg/actions/nodegroup/create.go +++ b/pkg/actions/nodegroup/create.go @@ -183,7 +183,7 @@ func (m *Manager) nodeCreationTasks(supportsManagedNodes, isOwnedCluster bool) e if nodeGroupTasks.Len() > 0 { allNodeGroupTasks.Append(nodeGroupTasks) } - managedTasks := m.stackManager.NewManagedNodeGroupTask(cfg.ManagedNodeGroups, !awsNodeUsesIRSA, vpcImporter, isOwnedCluster) + managedTasks := m.stackManager.NewManagedNodeGroupTask(cfg.ManagedNodeGroups, !awsNodeUsesIRSA, vpcImporter) if managedTasks.Len() > 0 { allNodeGroupTasks.Append(managedTasks) } diff --git a/pkg/cfn/manager/api.go b/pkg/cfn/manager/api.go index 53bbb4923e..18d3cbc4a0 100644 --- a/pkg/cfn/manager/api.go +++ b/pkg/cfn/manager/api.go @@ -9,13 +9,13 @@ import ( "github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface" "github.com/aws/aws-sdk-go/service/cloudtrail/cloudtrailiface" - "github.com/weaveworks/eksctl/pkg/cfn/waiter" - "github.com/aws/aws-sdk-go/service/cloudformation/cloudformationiface" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" "github.com/aws/aws-sdk-go/service/eks/eksiface" "github.com/aws/aws-sdk-go/service/iam/iamiface" + "github.com/weaveworks/eksctl/pkg/cfn/waiter" + "github.com/weaveworks/eksctl/pkg/version" "github.com/aws/aws-sdk-go/aws" diff --git a/pkg/cfn/manager/create_tasks.go b/pkg/cfn/manager/create_tasks.go index d54762533e..3d1345c682 100644 --- a/pkg/cfn/manager/create_tasks.go +++ b/pkg/cfn/manager/create_tasks.go @@ -37,7 +37,7 @@ func (c *StackCollection) NewTasksToCreateClusterWithNodeGroups(nodeGroups []*ap appendNodeGroupTasksTo := func(taskTree *tasks.TaskTree) { vpcImporter := vpc.NewStackConfigImporter(c.MakeClusterStackName()) nodeGroupTasks := c.NewUnmanagedNodeGroupTask(nodeGroups, false, vpcImporter) - managedNodeGroupTasks := c.NewManagedNodeGroupTask(managedNodeGroups, false, vpcImporter, true) + managedNodeGroupTasks := c.NewManagedNodeGroupTask(managedNodeGroups, false, vpcImporter) if managedNodeGroupTasks.Len() > 0 { nodeGroupTasks.Append(managedNodeGroupTasks.Tasks...) } @@ -82,7 +82,7 @@ func (c *StackCollection) NewUnmanagedNodeGroupTask(nodeGroups []*api.NodeGroup, } // NewManagedNodeGroupTask defines tasks required to create managed nodegroups -func (c *StackCollection) NewManagedNodeGroupTask(nodeGroups []*api.ManagedNodeGroup, forceAddCNIPolicy bool, vpcImporter vpc.Importer, isOwnedCluster bool) *tasks.TaskTree { +func (c *StackCollection) NewManagedNodeGroupTask(nodeGroups []*api.ManagedNodeGroup, forceAddCNIPolicy bool, vpcImporter vpc.Importer) *tasks.TaskTree { taskTree := &tasks.TaskTree{Parallel: true} for _, ng := range nodeGroups { taskTree.Append(&managedNodeGroupTask{ @@ -90,7 +90,6 @@ func (c *StackCollection) NewManagedNodeGroupTask(nodeGroups []*api.ManagedNodeG nodeGroup: ng, forceAddCNIPolicy: forceAddCNIPolicy, vpcImporter: vpcImporter, - isOwnedCluster: isOwnedCluster, info: fmt.Sprintf("create managed nodegroup %q", ng.Name), }) } diff --git a/pkg/cfn/manager/fakes/fake_stack_manager.go b/pkg/cfn/manager/fakes/fake_stack_manager.go index f4b44d9145..4a5b102d9c 100644 --- a/pkg/cfn/manager/fakes/fake_stack_manager.go +++ b/pkg/cfn/manager/fakes/fake_stack_manager.go @@ -522,13 +522,12 @@ type FakeStackManager struct { newClusterCompatTaskReturnsOnCall map[int]struct { result1 tasks.Task } - NewManagedNodeGroupTaskStub func([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer, bool) *tasks.TaskTree + NewManagedNodeGroupTaskStub func([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer) *tasks.TaskTree newManagedNodeGroupTaskMutex sync.RWMutex newManagedNodeGroupTaskArgsForCall []struct { arg1 []*v1alpha5.ManagedNodeGroup arg2 bool arg3 vpc.Importer - arg4 bool } newManagedNodeGroupTaskReturns struct { result1 *tasks.TaskTree @@ -3197,7 +3196,7 @@ func (fake *FakeStackManager) NewClusterCompatTaskReturnsOnCall(i int, result1 t }{result1} } -func (fake *FakeStackManager) NewManagedNodeGroupTask(arg1 []*v1alpha5.ManagedNodeGroup, arg2 bool, arg3 vpc.Importer, arg4 bool) *tasks.TaskTree { +func (fake *FakeStackManager) NewManagedNodeGroupTask(arg1 []*v1alpha5.ManagedNodeGroup, arg2 bool, arg3 vpc.Importer) *tasks.TaskTree { var arg1Copy []*v1alpha5.ManagedNodeGroup if arg1 != nil { arg1Copy = make([]*v1alpha5.ManagedNodeGroup, len(arg1)) @@ -3209,14 +3208,13 @@ func (fake *FakeStackManager) NewManagedNodeGroupTask(arg1 []*v1alpha5.ManagedNo arg1 []*v1alpha5.ManagedNodeGroup arg2 bool arg3 vpc.Importer - arg4 bool - }{arg1Copy, arg2, arg3, arg4}) + }{arg1Copy, arg2, arg3}) stub := fake.NewManagedNodeGroupTaskStub fakeReturns := fake.newManagedNodeGroupTaskReturns - fake.recordInvocation("NewManagedNodeGroupTask", []interface{}{arg1Copy, arg2, arg3, arg4}) + fake.recordInvocation("NewManagedNodeGroupTask", []interface{}{arg1Copy, arg2, arg3}) fake.newManagedNodeGroupTaskMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3, arg4) + return stub(arg1, arg2, arg3) } if specificReturn { return ret.result1 @@ -3230,17 +3228,17 @@ func (fake *FakeStackManager) NewManagedNodeGroupTaskCallCount() int { return len(fake.newManagedNodeGroupTaskArgsForCall) } -func (fake *FakeStackManager) NewManagedNodeGroupTaskCalls(stub func([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer, bool) *tasks.TaskTree) { +func (fake *FakeStackManager) NewManagedNodeGroupTaskCalls(stub func([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer) *tasks.TaskTree) { fake.newManagedNodeGroupTaskMutex.Lock() defer fake.newManagedNodeGroupTaskMutex.Unlock() fake.NewManagedNodeGroupTaskStub = stub } -func (fake *FakeStackManager) NewManagedNodeGroupTaskArgsForCall(i int) ([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer, bool) { +func (fake *FakeStackManager) NewManagedNodeGroupTaskArgsForCall(i int) ([]*v1alpha5.ManagedNodeGroup, bool, vpc.Importer) { fake.newManagedNodeGroupTaskMutex.RLock() defer fake.newManagedNodeGroupTaskMutex.RUnlock() argsForCall := fake.newManagedNodeGroupTaskArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } func (fake *FakeStackManager) NewManagedNodeGroupTaskReturns(result1 *tasks.TaskTree) { diff --git a/pkg/cfn/manager/interface.go b/pkg/cfn/manager/interface.go index 5bb3565fc0..acd3ef59c0 100644 --- a/pkg/cfn/manager/interface.go +++ b/pkg/cfn/manager/interface.go @@ -55,7 +55,7 @@ type StackManager interface { NewTasksToCreateClusterWithNodeGroups(nodeGroups []*v1alpha5.NodeGroup, managedNodeGroups []*v1alpha5.ManagedNodeGroup, supportsManagedNodes bool, postClusterCreationTasks ...tasks.Task) *tasks.TaskTree NewUnmanagedNodeGroupTask(nodeGroups []*v1alpha5.NodeGroup, forceAddCNIPolicy bool, importer vpc.Importer) *tasks.TaskTree - NewManagedNodeGroupTask(nodeGroups []*v1alpha5.ManagedNodeGroup, forceAddCNIPolicy bool, importer vpc.Importer, isOwnedCluster bool) *tasks.TaskTree + NewManagedNodeGroupTask(nodeGroups []*v1alpha5.ManagedNodeGroup, forceAddCNIPolicy bool, importer vpc.Importer) *tasks.TaskTree NewClusterCompatTask() tasks.Task NewTasksToCreateIAMServiceAccounts(serviceAccounts []*v1alpha5.ClusterIAMServiceAccount, oidc *iamoidc.OpenIDConnectManager, clientSetGetter kubernetes.ClientSetGetter) *tasks.TaskTree DeleteTasksForDeprecatedStacks() (*tasks.TaskTree, error) diff --git a/pkg/cfn/manager/nodegroup.go b/pkg/cfn/manager/nodegroup.go index 5d88d5f565..660cc8d93d 100644 --- a/pkg/cfn/manager/nodegroup.go +++ b/pkg/cfn/manager/nodegroup.go @@ -79,9 +79,13 @@ func (c *StackCollection) createNodeGroupTask(errs chan error, ng *api.NodeGroup return c.CreateStack(name, stack, ng.Tags, nil, errs) } -func (c *StackCollection) createManagedNodeGroupTask(errorCh chan error, ng *api.ManagedNodeGroup, forceAddCNIPolicy bool, vpcImporter vpc.Importer, isOwnedCluster bool) error { +func (c *StackCollection) createManagedNodeGroupTask(errorCh chan error, ng *api.ManagedNodeGroup, forceAddCNIPolicy bool, vpcImporter vpc.Importer) error { name := c.makeNodeGroupStackName(ng.Name) - + cluster, err := c.DescribeClusterStack() + if err != nil { + return err + } + isOwnedCluster := cluster != nil logger.Info("building managed nodegroup stack %q", name) bootstrapper := nodebootstrap.NewManagedBootstrapper(c.spec, ng) stack := builder.NewManagedNodeGroup(c.ec2API, c.spec, ng, builder.NewLaunchTemplateFetcher(c.ec2API), bootstrapper, forceAddCNIPolicy, vpcImporter, isOwnedCluster) diff --git a/pkg/cfn/manager/tasks.go b/pkg/cfn/manager/tasks.go index b89318a4cf..a4f1a36c03 100644 --- a/pkg/cfn/manager/tasks.go +++ b/pkg/cfn/manager/tasks.go @@ -43,13 +43,12 @@ type managedNodeGroupTask struct { stackCollection *StackCollection forceAddCNIPolicy bool vpcImporter vpc.Importer - isOwnedCluster bool } func (t *managedNodeGroupTask) Describe() string { return t.info } func (t *managedNodeGroupTask) Do(errorCh chan error) error { - return t.stackCollection.createManagedNodeGroupTask(errorCh, t.nodeGroup, t.forceAddCNIPolicy, t.vpcImporter, t.isOwnedCluster) + return t.stackCollection.createManagedNodeGroupTask(errorCh, t.nodeGroup, t.forceAddCNIPolicy, t.vpcImporter) } type clusterCompatTask struct { diff --git a/pkg/eks/mocks/KubeNodeGroup.go b/pkg/eks/mocks/KubeNodeGroup.go index c4714e2b50..49e2ea4f2b 100644 --- a/pkg/eks/mocks/KubeNodeGroup.go +++ b/pkg/eks/mocks/KubeNodeGroup.go @@ -1,4 +1,4 @@ -// Code generated by mockery v0.0.0-dev. DO NOT EDIT. +// Code generated by mockery v1.0.0. DO NOT EDIT. package mocks From 20d15699f03eb83ed3f603f85fc6fa4a0a75c0ba Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Fri, 15 Oct 2021 10:49:58 +0200 Subject: [PATCH 11/12] Shifted the logic into the task building and removed the extra parameter from the builder --- .../builder/managed_launch_template_test.go | 8 ++-- pkg/cfn/builder/managed_nodegroup.go | 8 +--- pkg/cfn/builder/managed_nodegroup_test.go | 26 +++---------- pkg/cfn/builder/nodegroup.go | 1 + pkg/cfn/manager/api.go | 3 +- pkg/cfn/manager/create_tasks.go | 3 +- pkg/cfn/manager/interface.go | 1 - pkg/cfn/manager/nodegroup.go | 7 +++- pkg/cfn/manager/tasks_test.go | 39 +++++++++++++++++++ userdocs/src/usage/vpc-networking.md | 2 +- 10 files changed, 57 insertions(+), 41 deletions(-) diff --git a/pkg/cfn/builder/managed_launch_template_test.go b/pkg/cfn/builder/managed_launch_template_test.go index 76a4b6ff43..8be5098e4f 100644 --- a/pkg/cfn/builder/managed_launch_template_test.go +++ b/pkg/cfn/builder/managed_launch_template_test.go @@ -13,16 +13,14 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" - "github.com/weaveworks/eksctl/pkg/nodebootstrap/fakes" "github.com/stretchr/testify/mock" - "github.com/weaveworks/goformation/v4" - gfnt "github.com/weaveworks/goformation/v4/cloudformation/types" - api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" vpcfakes "github.com/weaveworks/eksctl/pkg/vpc/fakes" + "github.com/weaveworks/goformation/v4" + gfnt "github.com/weaveworks/goformation/v4/cloudformation/types" ) type mngCase struct { @@ -60,7 +58,7 @@ var _ = Describe("ManagedNodeGroup builder", func() { return userData, nil } - stack := NewManagedNodeGroup(provider.MockEC2(), clusterConfig, m.ng, NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, fakeVPCImporter, true) + stack := NewManagedNodeGroup(provider.MockEC2(), clusterConfig, m.ng, NewLaunchTemplateFetcher(provider.MockEC2()), bootstrapper, false, fakeVPCImporter) err := stack.AddAllResources() if m.errMsg != "" { Expect(err).To(HaveOccurred()) diff --git a/pkg/cfn/builder/managed_nodegroup.go b/pkg/cfn/builder/managed_nodegroup.go index 5ed3083e12..fe50c7f915 100644 --- a/pkg/cfn/builder/managed_nodegroup.go +++ b/pkg/cfn/builder/managed_nodegroup.go @@ -15,7 +15,6 @@ import ( api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/nodebootstrap" "github.com/weaveworks/eksctl/pkg/utils" - utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" "github.com/weaveworks/eksctl/pkg/vpc" ) @@ -28,14 +27,13 @@ type ManagedNodeGroupResourceSet struct { ec2API ec2iface.EC2API vpcImporter vpc.Importer bootstrapper nodebootstrap.Bootstrapper - isOwnedCluster bool *resourceSet } const ManagedNodeGroupResourceName = "ManagedNodeGroup" // NewManagedNodeGroup creates a new ManagedNodeGroupResourceSet -func NewManagedNodeGroup(ec2API ec2iface.EC2API, cluster *api.ClusterConfig, nodeGroup *api.ManagedNodeGroup, launchTemplateFetcher *LaunchTemplateFetcher, bootstrapper nodebootstrap.Bootstrapper, forceAddCNIPolicy bool, vpcImporter vpc.Importer, isOwnedCluster bool) *ManagedNodeGroupResourceSet { +func NewManagedNodeGroup(ec2API ec2iface.EC2API, cluster *api.ClusterConfig, nodeGroup *api.ManagedNodeGroup, launchTemplateFetcher *LaunchTemplateFetcher, bootstrapper nodebootstrap.Bootstrapper, forceAddCNIPolicy bool, vpcImporter vpc.Importer) *ManagedNodeGroupResourceSet { return &ManagedNodeGroupResourceSet{ clusterConfig: cluster, forceAddCNIPolicy: forceAddCNIPolicy, @@ -45,15 +43,11 @@ func NewManagedNodeGroup(ec2API ec2iface.EC2API, cluster *api.ClusterConfig, nod resourceSet: newResourceSet(), vpcImporter: vpcImporter, bootstrapper: bootstrapper, - isOwnedCluster: isOwnedCluster, } } // AddAllResources adds all required CloudFormation resources func (m *ManagedNodeGroupResourceSet) AddAllResources() error { - if utilsstrings.Value(m.clusterConfig.VPC.IPFamily) == string(api.IPV6Family) && !m.isOwnedCluster { - return errors.New("managed nodegroups cannot be created on IPv6 unowned clusters") - } m.resourceSet.template.Description = fmt.Sprintf( "%s (SSH access: %v) %s", "EKS Managed Nodes", diff --git a/pkg/cfn/builder/managed_nodegroup_test.go b/pkg/cfn/builder/managed_nodegroup_test.go index afe85d4663..03cd7c3bb1 100644 --- a/pkg/cfn/builder/managed_nodegroup_test.go +++ b/pkg/cfn/builder/managed_nodegroup_test.go @@ -5,16 +5,14 @@ import ( "testing" "github.com/stretchr/testify/require" - "github.com/weaveworks/goformation/v4" - gfneks "github.com/weaveworks/goformation/v4/cloudformation/eks" - gfnt "github.com/weaveworks/goformation/v4/cloudformation/types" - api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/nodebootstrap" "github.com/weaveworks/eksctl/pkg/nodebootstrap/fakes" "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" - utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" vpcfakes "github.com/weaveworks/eksctl/pkg/vpc/fakes" + "github.com/weaveworks/goformation/v4" + gfneks "github.com/weaveworks/goformation/v4/cloudformation/eks" + gfnt "github.com/weaveworks/goformation/v4/cloudformation/types" ) func TestManagedPolicyResources(t *testing.T) { @@ -91,7 +89,7 @@ func TestManagedPolicyResources(t *testing.T) { bootstrapper.UserDataStub = func() (string, error) { return "", nil } - stack := NewManagedNodeGroup(p.EC2(), clusterConfig, ng, nil, bootstrapper, false, fakeVPCImporter, true) + stack := NewManagedNodeGroup(p.EC2(), clusterConfig, ng, nil, bootstrapper, false, fakeVPCImporter) err := stack.AddAllResources() require.Nil(err) @@ -160,7 +158,7 @@ func TestManagedNodeRole(t *testing.T) { p := mockprovider.NewMockProvider() fakeVPCImporter := new(vpcfakes.FakeImporter) bootstrapper := nodebootstrap.NewManagedBootstrapper(clusterConfig, tt.nodeGroup) - stack := NewManagedNodeGroup(p.EC2(), clusterConfig, tt.nodeGroup, nil, bootstrapper, false, fakeVPCImporter, true) + stack := NewManagedNodeGroup(p.EC2(), clusterConfig, tt.nodeGroup, nil, bootstrapper, false, fakeVPCImporter) err := stack.AddAllResources() require.NoError(err) @@ -181,20 +179,6 @@ func TestManagedNodeRole(t *testing.T) { } } -func TestManagedNodeGroupAddAllResources_WithUnownedIPv6Cluster(t *testing.T) { - ng := &api.ManagedNodeGroup{ - NodeGroupBase: &api.NodeGroupBase{}, - } - p := mockprovider.NewMockProvider() - clusterConfig := api.NewClusterConfig() - clusterConfig.VPC.IPFamily = utilsstrings.Pointer(string(api.IPV6Family)) - fakeVPCImporter := new(vpcfakes.FakeImporter) - bootstrapper := nodebootstrap.NewManagedBootstrapper(clusterConfig, ng) - stack := NewManagedNodeGroup(p.EC2(), clusterConfig, ng, nil, bootstrapper, false, fakeVPCImporter, false) - err := stack.AddAllResources() - require.EqualError(t, err, "managed nodegroups cannot be created on IPv6 unowned clusters") -} - func makePartitionedPolicies(policies ...string) []*gfnt.Value { var partitionedPolicies []*gfnt.Value for _, policy := range policies { diff --git a/pkg/cfn/builder/nodegroup.go b/pkg/cfn/builder/nodegroup.go index e5f9dad7b2..ce14d7d651 100644 --- a/pkg/cfn/builder/nodegroup.go +++ b/pkg/cfn/builder/nodegroup.go @@ -52,6 +52,7 @@ func NewNodeGroupResourceSet(ec2API ec2iface.EC2API, iamAPI iamiface.IAMAPI, spe // AddAllResources adds all the information about the nodegroup to the resource set func (n *NodeGroupResourceSet) AddAllResources() error { + if utilsstrings.Value(n.clusterSpec.VPC.IPFamily) == string(api.IPV6Family) { return errors.New("unmanaged nodegroups are not supported with IPv6 clusters") } diff --git a/pkg/cfn/manager/api.go b/pkg/cfn/manager/api.go index 18d3cbc4a0..ae177e89eb 100644 --- a/pkg/cfn/manager/api.go +++ b/pkg/cfn/manager/api.go @@ -8,14 +8,13 @@ import ( "github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface" "github.com/aws/aws-sdk-go/service/cloudtrail/cloudtrailiface" + "github.com/weaveworks/eksctl/pkg/cfn/waiter" "github.com/aws/aws-sdk-go/service/cloudformation/cloudformationiface" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" "github.com/aws/aws-sdk-go/service/eks/eksiface" "github.com/aws/aws-sdk-go/service/iam/iamiface" - "github.com/weaveworks/eksctl/pkg/cfn/waiter" - "github.com/weaveworks/eksctl/pkg/version" "github.com/aws/aws-sdk-go/aws" diff --git a/pkg/cfn/manager/create_tasks.go b/pkg/cfn/manager/create_tasks.go index 3d1345c682..a038e31aea 100644 --- a/pkg/cfn/manager/create_tasks.go +++ b/pkg/cfn/manager/create_tasks.go @@ -5,13 +5,12 @@ import ( "github.com/kris-nova/logger" "github.com/pkg/errors" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" iamoidc "github.com/weaveworks/eksctl/pkg/iam/oidc" "github.com/weaveworks/eksctl/pkg/kubernetes" "github.com/weaveworks/eksctl/pkg/utils/tasks" "github.com/weaveworks/eksctl/pkg/vpc" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( diff --git a/pkg/cfn/manager/interface.go b/pkg/cfn/manager/interface.go index acd3ef59c0..4bdf4b9c17 100644 --- a/pkg/cfn/manager/interface.go +++ b/pkg/cfn/manager/interface.go @@ -4,7 +4,6 @@ import ( "github.com/aws/aws-sdk-go/service/cloudformation" "github.com/aws/aws-sdk-go/service/cloudtrail" "github.com/aws/aws-sdk-go/service/eks/eksiface" - "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/cfn/builder" iamoidc "github.com/weaveworks/eksctl/pkg/iam/oidc" diff --git a/pkg/cfn/manager/nodegroup.go b/pkg/cfn/manager/nodegroup.go index 660cc8d93d..2140541514 100644 --- a/pkg/cfn/manager/nodegroup.go +++ b/pkg/cfn/manager/nodegroup.go @@ -15,6 +15,7 @@ import ( "github.com/tidwall/gjson" "github.com/weaveworks/eksctl/pkg/nodebootstrap" + utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/cfn/builder" @@ -85,10 +86,12 @@ func (c *StackCollection) createManagedNodeGroupTask(errorCh chan error, ng *api if err != nil { return err } - isOwnedCluster := cluster != nil + if cluster == nil && utilsstrings.Value(c.spec.VPC.IPFamily) == string(api.IPV6Family) { + return errors.New("managed nodegroups cannot be created on IPv6 unowned clusters") + } logger.Info("building managed nodegroup stack %q", name) bootstrapper := nodebootstrap.NewManagedBootstrapper(c.spec, ng) - stack := builder.NewManagedNodeGroup(c.ec2API, c.spec, ng, builder.NewLaunchTemplateFetcher(c.ec2API), bootstrapper, forceAddCNIPolicy, vpcImporter, isOwnedCluster) + stack := builder.NewManagedNodeGroup(c.ec2API, c.spec, ng, builder.NewLaunchTemplateFetcher(c.ec2API), bootstrapper, forceAddCNIPolicy, vpcImporter) if err := stack.AddAllResources(); err != nil { return err } diff --git a/pkg/cfn/manager/tasks_test.go b/pkg/cfn/manager/tasks_test.go index 6907a1e44d..f4008ddfed 100644 --- a/pkg/cfn/manager/tasks_test.go +++ b/pkg/cfn/manager/tasks_test.go @@ -5,8 +5,12 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/pkg/errors" + "github.com/stretchr/testify/mock" + api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" + utilsstrings "github.com/weaveworks/eksctl/pkg/utils/strings" vpcfakes "github.com/weaveworks/eksctl/pkg/vpc/fakes" ) @@ -135,4 +139,39 @@ var _ = Describe("StackCollection Tasks", func() { }) }) }) + Describe("ManagedNodeGroupTask", func() { + Context("ipv6 cluster", func() { + var ( + p *mockprovider.MockProvider + cfg *api.ClusterConfig + stackManager *StackCollection + ) + BeforeEach(func() { + p = mockprovider.NewMockProvider() + cfg = newClusterConfig("test-ipv6-cluster") + cfg.VPC.IPFamily = utilsstrings.Pointer(string(api.IPV6Family)) + stackManager = NewStackCollection(p, cfg) + }) + It("returns an error", func() { + p.MockCloudFormation().On("ListStacksPages", mock.Anything, mock.Anything).Return(nil) + ng := api.NewManagedNodeGroup() + fakeVPCImporter := new(vpcfakes.FakeImporter) + tasks := stackManager.NewManagedNodeGroupTask([]*api.ManagedNodeGroup{ng}, false, fakeVPCImporter) + errs := tasks.DoAllSync() + Expect(errs).To(HaveLen(1)) + Expect(errs[0]).To(MatchError(ContainSubstring("managed nodegroups cannot be created on IPv6 unowned clusters"))) + }) + When("finding the stack fails", func() { + It("returns the stack error", func() { + p.MockCloudFormation().On("ListStacksPages", mock.Anything, mock.Anything).Return(errors.New("not found")) + ng := api.NewManagedNodeGroup() + fakeVPCImporter := new(vpcfakes.FakeImporter) + tasks := stackManager.NewManagedNodeGroupTask([]*api.ManagedNodeGroup{ng}, false, fakeVPCImporter) + errs := tasks.DoAllSync() + Expect(errs).To(HaveLen(1)) + Expect(errs[0]).To(MatchError(ContainSubstring("not found"))) + }) + }) + }) + }) }) diff --git a/userdocs/src/usage/vpc-networking.md b/userdocs/src/usage/vpc-networking.md index 879eb9323f..73ecb0fc6b 100644 --- a/userdocs/src/usage/vpc-networking.md +++ b/userdocs/src/usage/vpc-networking.md @@ -57,7 +57,7 @@ This is an in config file setting only. When IPv6 is set, the following restrict - managed addons are defined as shows above - version must be => 1.21 - unmanaged nodegroups are not yet supported with IPv6 clusters -- nodegroup creation is not supported with un-owned IPv6 clusters +- managed nodegroup creation is not supported with un-owned IPv6 clusters - `vpc.NAT` and `serviceIPv4CIDR` fields are created by eksctl for ipv6 clusters and thus, are not supported configuration options - AutoAllocateIPv6 is not supported together with IPv6 From 43fdc0a27910efa4ebca6116d88fb218f6e08a18 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Tue, 19 Oct 2021 14:36:35 +0200 Subject: [PATCH 12/12] Update pkg/cfn/manager/tasks_test.go Co-authored-by: Jake Klein --- pkg/cfn/manager/tasks_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cfn/manager/tasks_test.go b/pkg/cfn/manager/tasks_test.go index f4008ddfed..37b6481398 100644 --- a/pkg/cfn/manager/tasks_test.go +++ b/pkg/cfn/manager/tasks_test.go @@ -140,7 +140,7 @@ var _ = Describe("StackCollection Tasks", func() { }) }) Describe("ManagedNodeGroupTask", func() { - Context("ipv6 cluster", func() { + When("creating managed nodegroups on a ipv6 cluster", func() { var ( p *mockprovider.MockProvider cfg *api.ClusterConfig