Do not wait for aws-ebs-csi-driver if there are no nodegroups #5929

radTuti · 2022-11-14T21:35:35Z

Description

aws-ebs-csi-driver will stay in degraded state until nodegroups are added.
SInce the addon is now a requirement for k8s 1.23, the cluster creation should not fail.

Manual validation

Given cluster config cluster.yaml

kind: ClusterConfig
apiVersion: eksctl.io/v1alpha5

metadata:
  name: cluster-1jnq
  region: us-west-2
  version: "1.23"

iam:
  withOIDC: true
  serviceAccounts:
  - metadata:
      name: ebs-csi-controller-sa
      namespace: kube-system
    attachPolicyARNs:
    - "arn:aws:iam::aws:policy/service-role/AmazonEBSCSIDriverPolicy"
    roleOnly: true
    roleName: cluster-1jnq-AmazonEKS_EBS_CSI_DriverRole

addons:
- name: aws-ebs-csi-driver
  version: v1.11.4-eksbuild.1
  serviceAccountRoleARN: arn:aws:iam::ACCOUNT_ID:role/cluster-1jnq-AmazonEKS_EBS_CSI_DriverRole

nodeGroups:
  - name: worker-pool
    instanceType: t3.large
    desiredCapacity: 3

Running eksctl create cluster -f cluster.yaml --without-nodegroup successfully creates the cluster with no node group.

Additional validated scenarios:

setting desiredCapacity: 0 in cluster.yaml and running eksctl create cluster -f cluster.yaml
setting desiredCapacity:0, maxSize: 3 in cluster.yaml and running eksctl create cluster -f cluster.yaml

Checklist

Added tests that cover your change (if possible)
~~Added/modified documentation as required (such as the README.md, or the userdocs directory)~~
Manually tested
Made sure the title of the PR is a good description that can go into the release notes
(Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Backfilled missing tests for code in same general area 🎉
Refactored something and made the world a better place 🌟

github-actions

Hello radTuti 👋 Thank you for opening a Pull Request in eksctl project. The team will review the Pull Request and aim to respond within 1-10 business days. Meanwhile, please read about the Contribution and Code of Conduct guidelines here. You can find out more information about eksctl on our website

cPu1

Thanks for the detailed PR. I have left some suggestions on improving the tests but otherwise it LGTM!

cPu1 · 2022-11-21T09:21:40Z

pkg/actions/addon/addon.go

+var (
+	ExportWaitForAddonToBeActive = (*Manager).waitForAddonToBeActive
+)


We don't want this exposed outside of this package, especially since there's an alternative. You could test this case by calling manager.Create, passing a non-zero waitTimeout. You'll have to mock the Addons API though. There are examples in pkg/actions/addon/create_test.go.

Suggested change

var (

ExportWaitForAddonToBeActive = (*Manager).waitForAddonToBeActive

)

cPu1 · 2022-11-21T09:23:55Z

pkg/actions/addon/addon_test.go

+	var (
+		clusterConfig *api.ClusterConfig
+		mockProvider  *mockprovider.MockProvider
+		manager       *addon.Manager
+	)
+
+	BeforeEach(func() {
+		var err error
+		clusterConfig = &api.ClusterConfig{Metadata: &api.ClusterMeta{
+			Version: "1.18",
+			Name:    "my-cluster",
+		}}
+		mockProvider = mockprovider.NewMockProvider()
+		manager, err = addon.New(clusterConfig, mockProvider.EKS(), nil, false, nil, nil)
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	When("addon is coredns with no nodegroup", func() {
+		It("does not wait for addon to be active", func() {
+			a := &api.Addon{
+				Name: api.CoreDNSAddon,
+			}
+			err := addon.ExportWaitForAddonToBeActive(manager, context.Background(), a, 0)
+			Expect(err).To(BeNil())
+		})
+	})
+
+	When("addon is aws-ebs-csi-driver with no nodegroup", func() {
+		It("does not wait for addon to be active", func() {
+			a := &api.Addon{
+				Name: api.AWSEBSCSIDriverAddon,
+			}
+			err := addon.ExportWaitForAddonToBeActive(manager, context.Background(), a, 0)
+			Expect(err).To(BeNil())


This should go in pkg/actions/addon/create_test.go and, as noted in the comment above, it should be tested by calling manager.Create.

cPu1 · 2022-11-21T09:24:34Z

pkg/actions/addon/create.go

-	vpcCNIName          = "vpc-cni"
-	ebsCSIDriverName    = "aws-ebs-csi-driver"


radTuti · 2022-11-21T20:09:02Z

Thanks for the detailed PR. I have left some suggestions on improving the tests but otherwise it LGTM!

Thanks for the review and updating the tests. I have updated the branch. Is there anything else needed to get this merged?

github-actions bot reviewed Nov 14, 2022

View reviewed changes

radTuti force-pushed the addon-deployment-no-nodegroup branch 2 times, most recently from db73b4f to 6d86abd Compare November 15, 2022 17:00

cPu1 added the kind/bug label Nov 21, 2022

cPu1 reviewed Nov 21, 2022

View reviewed changes

cPu1 self-assigned this Nov 21, 2022

radTuti added 2 commits November 21, 2022 12:08

do not wait for aws-ebs-csi-driver addon if there are no nodegroups

a80372b

clean up references to addon names

708c544

radTuti force-pushed the addon-deployment-no-nodegroup branch from e092b8c to c2b4e08 Compare November 21, 2022 20:08

cPu1 force-pushed the addon-deployment-no-nodegroup branch from c2b4e08 to 708c544 Compare November 22, 2022 08:27

Add unit tests for CoreDNS and AWS EBS CSI driver addons

903f1eb

cPu1 approved these changes Nov 22, 2022

View reviewed changes

cPu1 merged commit 3ed3892 into eksctl-io:main Nov 22, 2022

kryczech mentioned this pull request Aug 11, 2023

[Bug] Cluster creation fails if no nodegroups and EFS csi addon required #6954

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not wait for aws-ebs-csi-driver if there are no nodegroups #5929

Do not wait for aws-ebs-csi-driver if there are no nodegroups #5929

radTuti commented Nov 14, 2022

github-actions bot left a comment

cPu1 left a comment

cPu1 Nov 21, 2022

cPu1 Nov 21, 2022

cPu1 Nov 21, 2022

radTuti commented Nov 21, 2022

	var (
	ExportWaitForAddonToBeActive = (*Manager).waitForAddonToBeActive
	)

		vpcCNIName = "vpc-cni"
		ebsCSIDriverName = "aws-ebs-csi-driver"

Do not wait for aws-ebs-csi-driver if there are no nodegroups #5929

Do not wait for aws-ebs-csi-driver if there are no nodegroups #5929

Conversation

radTuti commented Nov 14, 2022

Description

Manual validation

Checklist

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

github-actions bot left a comment

Choose a reason for hiding this comment

cPu1 left a comment

Choose a reason for hiding this comment

cPu1 Nov 21, 2022

Choose a reason for hiding this comment

cPu1 Nov 21, 2022

Choose a reason for hiding this comment

cPu1 Nov 21, 2022

Choose a reason for hiding this comment

radTuti commented Nov 21, 2022