From b7bf1ac67128116330b162076b25c86e1c6e86fc Mon Sep 17 00:00:00 2001 From: sushrk Date: Fri, 27 Sep 2024 01:42:21 +0000 Subject: [PATCH 1/4] add CNINode integration tests --- pkg/utils/helper.go | 3 +- pkg/utils/set.go | 12 +++ scripts/test/run-integration-tests.sh | 2 +- test/framework/framework.go | 63 ++++++------ .../resource/aws/autoscaling/manager.go | 64 +++++++++++++ test/framework/resource/k8s/node/manager.go | 10 ++ test/framework/resource/k8s/node/wrapper.go | 57 ++++++++--- .../integration/cninode/cninode_suite_test.go | 50 ++++++++++ test/integration/cninode/cninode_test.go | 95 +++++++++++++++++++ .../perpodsg/perpodsg_suite_test.go | 4 +- test/integration/perpodsg/perpodsg_test.go | 23 ----- .../integration/windows/windows_suite_test.go | 2 +- test/integration/windows/windows_test.go | 4 +- 13 files changed, 316 insertions(+), 73 deletions(-) create mode 100644 test/framework/resource/aws/autoscaling/manager.go create mode 100644 test/integration/cninode/cninode_suite_test.go create mode 100644 test/integration/cninode/cninode_test.go diff --git a/pkg/utils/helper.go b/pkg/utils/helper.go index 6b16d045..2a17b685 100644 --- a/pkg/utils/helper.go +++ b/pkg/utils/helper.go @@ -27,7 +27,6 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -237,7 +236,7 @@ func GetSourceAcctAndArn(roleARN, region, clusterName string) (string, string, s // PodHasENIRequest will return true if first container of pod spec has request for eni indicating // it needs trunk interface from vpc-rc -func PodHasENIRequest(pod *v1.Pod) bool { +func PodHasENIRequest(pod *corev1.Pod) bool { if pod == nil { return false } diff --git a/pkg/utils/set.go b/pkg/utils/set.go index ab7a037e..326ffa6d 100644 --- a/pkg/utils/set.go +++ b/pkg/utils/set.go @@ -13,6 +13,10 @@ package utils +import ( + "github.com/aws/aws-sdk-go/service/ec2" +) + // Difference returns a-b, elements present in a and not in b func Difference[T comparable](a, b []T) (diff []T) { m := make(map[T]struct{}) @@ -35,3 +39,11 @@ func GetKeyValSlice(m map[string]string) (key []string, val []string) { } return } + +func GetTagKeyValueMap(tagSet []*ec2.Tag) map[string]string { + m := make(map[string]string) + for _, tag := range tagSet { + m[*tag.Key] = *tag.Value + } + return m +} diff --git a/scripts/test/run-integration-tests.sh b/scripts/test/run-integration-tests.sh index b378a25e..7c370ea4 100755 --- a/scripts/test/run-integration-tests.sh +++ b/scripts/test/run-integration-tests.sh @@ -47,7 +47,7 @@ function run_integration_tests(){ echo "skipping Windows tests" fi (cd $INTEGRATION_TEST_DIR/webhook && CGO_ENABLED=0 ginkgo --skip=LOCAL $EXTRA_GINKGO_FLAGS -v -timeout=5m -- -cluster-kubeconfig=$KUBE_CONFIG_PATH -cluster-name=$CLUSTER_NAME --aws-region=$REGION --aws-vpc-id $VPC_ID) || TEST_RESULT=fail - # (cd $INTEGRATION_TEST_DIR/cninode && CGO_ENABLED=0 ginkgo --skip=LOCAL $EXTRA_GINKGO_FLAGS -v -timeout=10m -- -cluster-kubeconfig=$KUBE_CONFIG_PATH -cluster-name=$CLUSTER_NAME --aws-region=$REGION --aws-vpc-id $VPC_ID) || TEST_RESULT=fail + (cd $INTEGRATION_TEST_DIR/cninode && CGO_ENABLED=0 ginkgo --skip=LOCAL $EXTRA_GINKGO_FLAGS -v -timeout=10m -- -cluster-kubeconfig=$KUBE_CONFIG_PATH -cluster-name=$CLUSTER_NAME --aws-region=$REGION --aws-vpc-id $VPC_ID) || TEST_RESULT=fail if [[ "$TEST_RESULT" == fail ]]; then exit 1 diff --git a/test/framework/framework.go b/test/framework/framework.go index f2f65be2..af650606 100644 --- a/test/framework/framework.go +++ b/test/framework/framework.go @@ -17,6 +17,7 @@ import ( eniConfig "github.com/aws/amazon-vpc-cni-k8s/pkg/apis/crd/v1alpha1" cninode "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1alpha1" sgp "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1beta1" + "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/resource/aws/autoscaling" ec2Manager "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/resource/aws/ec2" "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/resource/k8s/configmap" "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/resource/k8s/controller" @@ -42,21 +43,22 @@ import ( ) type Framework struct { - Options Options - K8sClient client.Client - ec2Client *ec2.EC2 - DeploymentManager deployment.Manager - PodManager pod.Manager - EC2Manager *ec2Manager.Manager - SAManager serviceaccount.Manager - NSManager namespace.Manager - SGPManager *sgpManager.Manager - SVCManager service.Manager - JobManager jobs.Manager - NodeManager node.Manager - ControllerManager controller.Manager - RBACManager rbac.Manager - ConfigMapManager configmap.Manager + Options Options + K8sClient client.Client + ec2Client *ec2.EC2 + DeploymentManager deployment.Manager + PodManager pod.Manager + EC2Manager *ec2Manager.Manager + SAManager serviceaccount.Manager + NSManager namespace.Manager + SGPManager *sgpManager.Manager + SVCManager service.Manager + JobManager jobs.Manager + NodeManager node.Manager + ControllerManager controller.Manager + RBACManager rbac.Manager + ConfigMapManager configmap.Manager + AutoScalingManager autoscaling.Manager } func New(options Options) *Framework { @@ -91,20 +93,21 @@ func New(options Options) *Framework { ec2 := ec2.New(sess, &aws.Config{Region: aws.String(options.AWSRegion)}) return &Framework{ - K8sClient: k8sClient, - ec2Client: ec2, - PodManager: pod.NewManager(k8sClient, k8sSchema, config), - DeploymentManager: deployment.NewManager(k8sClient), - EC2Manager: ec2Manager.NewManager(ec2, options.AWSVPCID), - SAManager: serviceaccount.NewManager(k8sClient, config), - NSManager: namespace.NewManager(k8sClient), - SGPManager: sgpManager.NewManager(k8sClient), - SVCManager: service.NewManager(k8sClient), - JobManager: jobs.NewManager(k8sClient), - NodeManager: node.NewManager(k8sClient), - ControllerManager: controller.NewManager(k8sClient), - RBACManager: rbac.NewManager(k8sClient), - ConfigMapManager: configmap.NewManager(k8sClient), - Options: options, + K8sClient: k8sClient, + ec2Client: ec2, + PodManager: pod.NewManager(k8sClient, k8sSchema, config), + DeploymentManager: deployment.NewManager(k8sClient), + EC2Manager: ec2Manager.NewManager(ec2, options.AWSVPCID), + SAManager: serviceaccount.NewManager(k8sClient, config), + NSManager: namespace.NewManager(k8sClient), + SGPManager: sgpManager.NewManager(k8sClient), + SVCManager: service.NewManager(k8sClient), + JobManager: jobs.NewManager(k8sClient), + NodeManager: node.NewManager(k8sClient), + ControllerManager: controller.NewManager(k8sClient), + RBACManager: rbac.NewManager(k8sClient), + ConfigMapManager: configmap.NewManager(k8sClient), + AutoScalingManager: autoscaling.NewManager(sess), + Options: options, } } diff --git a/test/framework/resource/aws/autoscaling/manager.go b/test/framework/resource/aws/autoscaling/manager.go new file mode 100644 index 00000000..4f3f1730 --- /dev/null +++ b/test/framework/resource/aws/autoscaling/manager.go @@ -0,0 +1,64 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package autoscaling + +import ( + "fmt" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/autoscaling" + "github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface" +) + +type Manager interface { + DescribeAutoScalingGroup(autoScalingGroupName string) ([]*autoscaling.Group, error) + UpdateAutoScalingGroup(asgName string, desiredSize, minSize, maxSize int64) error +} + +type defaultManager struct { + autoscalingiface.AutoScalingAPI +} + +func NewManager(session *session.Session) Manager { + return &defaultManager{ + AutoScalingAPI: autoscaling.New(session), + } +} + +func (d defaultManager) DescribeAutoScalingGroup(autoScalingGroupName string) ([]*autoscaling.Group, error) { + describeAutoScalingGroupIp := &autoscaling.DescribeAutoScalingGroupsInput{ + AutoScalingGroupNames: aws.StringSlice([]string{autoScalingGroupName}), + } + asg, err := d.AutoScalingAPI.DescribeAutoScalingGroups(describeAutoScalingGroupIp) + if err != nil { + return nil, err + } + if len(asg.AutoScalingGroups) == 0 { + return nil, fmt.Errorf("failed to find asg %s", autoScalingGroupName) + } + + return asg.AutoScalingGroups, nil +} + +func (d defaultManager) UpdateAutoScalingGroup(asgName string, desiredSize, minSize, maxSize int64) error { + updateASGInput := &autoscaling.UpdateAutoScalingGroupInput{ + AutoScalingGroupName: aws.String(asgName), + DesiredCapacity: aws.Int64(desiredSize), + MaxSize: aws.Int64(maxSize), + MinSize: aws.Int64(minSize), + } + _, err := d.AutoScalingAPI.UpdateAutoScalingGroup(updateASGInput) + return err +} diff --git a/test/framework/resource/k8s/node/manager.go b/test/framework/resource/k8s/node/manager.go index 6b3f42c9..c69c74c3 100644 --- a/test/framework/resource/k8s/node/manager.go +++ b/test/framework/resource/k8s/node/manager.go @@ -15,6 +15,7 @@ package node import ( "context" + "strings" cninode "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1alpha1" "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/utils" @@ -32,6 +33,7 @@ type Manager interface { GetNodeList() (*v1.NodeList, error) GetCNINode(node *v1.Node) (*cninode.CNINode, error) GetCNINodeList() (*cninode.CNINodeList, error) + GetInstanceID(node *v1.Node) string } type defaultManager struct { @@ -117,3 +119,11 @@ func (d *defaultManager) GetNodeList() (*v1.NodeList, error) { err := d.k8sClient.List(context.TODO(), list) return list, err } + +func (d *defaultManager) GetInstanceID(node *v1.Node) string { + if node.Spec.ProviderID != "" { + id := strings.Split(node.Spec.ProviderID, "/") + return id[len(id)-1] + } + return "" +} diff --git a/test/framework/resource/k8s/node/wrapper.go b/test/framework/resource/k8s/node/wrapper.go index e9cff281..e20ec448 100644 --- a/test/framework/resource/k8s/node/wrapper.go +++ b/test/framework/resource/k8s/node/wrapper.go @@ -15,30 +15,61 @@ package node import ( "context" + "fmt" + cninode "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1alpha1" "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/samber/lo" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/wait" ) -func GetNodeAndWaitTillCapacityPresent(manager Manager, ctx context.Context, os string, expectedResource string) *v1.NodeList { - +func GetNodeAndWaitTillCapacityPresent(manager Manager, os string, expectedResource string) *v1.NodeList { observedNodeList := &v1.NodeList{} var err error - err = wait.Poll(utils.PollIntervalShort, utils.ResourceCreationTimeout, func() (bool, error) { - By("checking nodes have capacity present") - observedNodeList, err = manager.GetNodesWithOS(os) - Expect(err).ToNot(HaveOccurred()) - for _, node := range observedNodeList.Items { - _, found := node.Status.Allocatable[v1.ResourceName(expectedResource)] - if !found { - return false, nil + err = wait.PollUntilContextTimeout(context.Background(), utils.PollIntervalShort, utils.ResourceCreationTimeout, true, + func(ctx context.Context) (bool, error) { + By("checking nodes have capacity present") + observedNodeList, err = manager.GetNodesWithOS(os) + Expect(err).ToNot(HaveOccurred()) + for _, node := range observedNodeList.Items { + _, found := node.Status.Allocatable[v1.ResourceName(expectedResource)] + if !found { + return false, nil + } } - } - return true, nil - }) + return true, nil + }) Expect(err).ToNot(HaveOccurred()) return observedNodeList } + +// VerifyCNINodeCount checks if the number of CNINodes is equal to number of nodes in the cluster, and verifies 1:1 mapping between CNINode and Node objects +// Returns nil if count and 1:1 mapping exists, else returns error +func VerifyCNINodeCount(manager Manager) error { + cniNodes, err := manager.GetCNINodeList() + Expect(err).NotTo(HaveOccurred()) + nodes, err := manager.GetNodeList() + Expect(err).NotTo(HaveOccurred()) + By("checking number of CNINodes match number of nodes in the cluster") + isEqual := len(nodes.Items) == len(cniNodes.Items) + if !isEqual { + return fmt.Errorf("number of CNINodes does not match number of nodes in the cluster") + } + + By("checking CNINode list matches node list") + nameMatched := true + for _, node := range nodes.Items { + if !lo.ContainsBy(cniNodes.Items, func(cniNode cninode.CNINode) bool { + return cniNode.Name == node.Name + }) { + nameMatched = false + } + } + if !nameMatched { + return fmt.Errorf("CNINode list does not match node list") + } + return nil +} diff --git a/test/integration/cninode/cninode_suite_test.go b/test/integration/cninode/cninode_suite_test.go new file mode 100644 index 00000000..cff568f1 --- /dev/null +++ b/test/integration/cninode/cninode_suite_test.go @@ -0,0 +1,50 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package cninode_test + +import ( + "testing" + + "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework" + "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/resource/k8s/node" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestCNINode(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "CNINode Test Suite") +} + +var frameWork *framework.Framework +var _ = BeforeSuite(func() { + By("creating a framework") + frameWork = framework.New(framework.GlobalOptions) + + By("verify at least 2 nodes are available") + nodeList, err := frameWork.NodeManager.GetNodeList() + Expect(err).ToNot(HaveOccurred()) + Expect(len(nodeList.Items)).To(BeNumerically(">", 1)) + + By("verify CNINode count") + err = node.VerifyCNINodeCount(frameWork.NodeManager) + Expect(err).ToNot(HaveOccurred()) +}) + +// Verify CNINode count before and after test remains same +var _ = AfterSuite(func() { + By("verify CNINode count") + err := node.VerifyCNINodeCount(frameWork.NodeManager) + Expect(err).ToNot(HaveOccurred()) +}) diff --git a/test/integration/cninode/cninode_test.go b/test/integration/cninode/cninode_test.go new file mode 100644 index 00000000..ad23f5ac --- /dev/null +++ b/test/integration/cninode/cninode_test.go @@ -0,0 +1,95 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package cninode_test + +import ( + "time" + + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils" + "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/resource/k8s/node" + testUtils "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/utils" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("[CANARY]CNINode test", func() { + Describe("CNINode count verification on adding or removing node", func() { + var oldDesiredSize int64 + var oldMinSize int64 + var oldMaxSize int64 + var newSize int64 + var asgName string + BeforeEach(func() { + By("getting autoscaling group name") + asgName = ListNodesAndGetAutoScalingGroupName() + asg, err := frameWork.AutoScalingManager.DescribeAutoScalingGroup(asgName) + Expect(err).ToNot(HaveOccurred()) + oldDesiredSize = *asg[0].DesiredCapacity + oldMinSize = *asg[0].MinSize + oldMaxSize = *asg[0].MaxSize + }) + AfterEach(func() { + By("restoring ASG minSize & maxSize after test") + err := frameWork.AutoScalingManager.UpdateAutoScalingGroup(asgName, oldDesiredSize, oldMinSize, oldMaxSize) + Expect(err).ToNot(HaveOccurred()) + // sleep to allow ASG to be updated + time.Sleep(testUtils.ResourceCreationTimeout) + }) + + Context("when new node is added", func() { + It("it should create new CNINode", func() { + newSize = oldDesiredSize + 1 + // Update ASG to set desiredSize + By("adding new node") + err := frameWork.AutoScalingManager.UpdateAutoScalingGroup(asgName, newSize, oldMinSize, newSize) + Expect(err).ToNot(HaveOccurred()) + // sleep to allow ASG to be updated + time.Sleep(testUtils.ResourceCreationTimeout) + + err = node.VerifyCNINodeCount(frameWork.NodeManager) + Expect(err).ToNot(HaveOccurred()) + }) + }) + Context("when existing node is removed", func() { + It("it should delete CNINode", func() { + newSize = oldDesiredSize - 1 + // Update ASG to set new minSize and new maxSize + By("removing existing node") + err := frameWork.AutoScalingManager.UpdateAutoScalingGroup(asgName, newSize, newSize, oldMaxSize) + Expect(err).ToNot(HaveOccurred()) + // sleep to allow ASG to be updated + time.Sleep(testUtils.ResourceCreationTimeout) + err = node.VerifyCNINodeCount(frameWork.NodeManager) + Expect(err).ToNot(HaveOccurred()) + + }) + }) + }) +}) + +func ListNodesAndGetAutoScalingGroupName() string { + By("getting instance details") + nodeList, err := frameWork.NodeManager.GetNodesWithOS(config.OSLinux) + Expect(err).ToNot(HaveOccurred()) + Expect(nodeList.Items).ToNot(BeEmpty()) + instanceID := frameWork.NodeManager.GetInstanceID(&nodeList.Items[0]) + Expect(instanceID).ToNot(BeEmpty()) + instance, err := frameWork.EC2Manager.GetInstanceDetails(instanceID) + Expect(err).ToNot(HaveOccurred()) + tags := utils.GetTagKeyValueMap(instance.Tags) + val, ok := tags["aws:autoscaling:groupName"] + Expect(ok).To(BeTrue()) + return val +} diff --git a/test/integration/perpodsg/perpodsg_suite_test.go b/test/integration/perpodsg/perpodsg_suite_test.go index c0e7c82e..7198297a 100644 --- a/test/integration/perpodsg/perpodsg_suite_test.go +++ b/test/integration/perpodsg/perpodsg_suite_test.go @@ -52,8 +52,10 @@ var _ = BeforeSuite(func() { securityGroupID2, err = frameWork.EC2Manager.ReCreateSG(utils.ResourceNamePrefix+"sg-2", ctx) Expect(err).ToNot(HaveOccurred()) - nodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, ctx, "linux", + nodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, "linux", config.ResourceNamePodENI) + err = node.VerifyCNINodeCount(frameWork.NodeManager) + Expect(err).ToNot(HaveOccurred()) }) var _ = AfterSuite(func() { diff --git a/test/integration/perpodsg/perpodsg_test.go b/test/integration/perpodsg/perpodsg_test.go index 4c34ddf8..ef8f51e4 100644 --- a/test/integration/perpodsg/perpodsg_test.go +++ b/test/integration/perpodsg/perpodsg_test.go @@ -37,29 +37,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -var _ = Describe("CNINode Veification", func() { - Describe("verify CNINode mapping to nodes", func() { - Context("when nodes are ready", func() { - It("should have same number of CNINode no matter which mode", func() { - cniNodes, err := frameWork.NodeManager.GetCNINodeList() - Expect(err).NotTo(HaveOccurred()) - nodes, err := frameWork.NodeManager.GetNodeList() - Expect(err).NotTo(HaveOccurred()) - Expect(len(nodes.Items)).To(Equal(len(cniNodes.Items))) - nameMatched := true - for _, node := range nodes.Items { - if !lo.ContainsBy(cniNodes.Items, func(cniNode cninode.CNINode) bool { - return cniNode.Name == node.Name - }) { - nameMatched = false - } - } - Expect(nameMatched).To(BeTrue()) - }) - }) - }) -}) - var _ = Describe("Branch ENI Pods", func() { var ( securityGroupPolicy *v1beta1.SecurityGroupPolicy diff --git a/test/integration/windows/windows_suite_test.go b/test/integration/windows/windows_suite_test.go index 60b147d1..bebd99eb 100644 --- a/test/integration/windows/windows_suite_test.go +++ b/test/integration/windows/windows_suite_test.go @@ -67,7 +67,7 @@ var _ = BeforeSuite(func() { } By("getting the list of Windows node") - windowsNodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, ctx, "windows", + windowsNodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, "windows", config.ResourceNameIPAddress) By("getting the instance ID for the first node") diff --git a/test/integration/windows/windows_test.go b/test/integration/windows/windows_test.go index dd96965b..ea85c479 100644 --- a/test/integration/windows/windows_test.go +++ b/test/integration/windows/windows_test.go @@ -228,7 +228,7 @@ var _ = Describe("Windows Integration Test", func() { } JustBeforeEach(func() { - windowsNodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, ctx, "windows", + windowsNodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, "windows", config.ResourceNameIPAddress) instanceID = manager.GetNodeInstanceID(&windowsNodeList.Items[0]) nodeName = windowsNodeList.Items[0].Name @@ -455,7 +455,7 @@ var _ = Describe("Windows Integration Test", func() { bufferForCoolDown = time.Second * 30 - windowsNodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, ctx, "windows", + windowsNodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, "windows", config.ResourceNameIPAddress) instanceID = manager.GetNodeInstanceID(&windowsNodeList.Items[0]) nodeName = windowsNodeList.Items[0].Name From 029d50a21e7175cbb3040f8e6ae5c0d9e7c3ac95 Mon Sep 17 00:00:00 2001 From: sushrk Date: Fri, 27 Sep 2024 23:05:55 +0000 Subject: [PATCH 2/4] address PR comments --- test/framework/resource/k8s/node/wrapper.go | 4 +-- .../integration/cninode/cninode_suite_test.go | 4 +-- test/integration/cninode/cninode_test.go | 36 ++++++++++++------- .../perpodsg/perpodsg_suite_test.go | 2 +- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/test/framework/resource/k8s/node/wrapper.go b/test/framework/resource/k8s/node/wrapper.go index e20ec448..fc54272d 100644 --- a/test/framework/resource/k8s/node/wrapper.go +++ b/test/framework/resource/k8s/node/wrapper.go @@ -46,9 +46,9 @@ func GetNodeAndWaitTillCapacityPresent(manager Manager, os string, expectedResou return observedNodeList } -// VerifyCNINodeCount checks if the number of CNINodes is equal to number of nodes in the cluster, and verifies 1:1 mapping between CNINode and Node objects +// VerifyCNINode checks if the number of CNINodes is equal to number of nodes in the cluster, and verifies 1:1 mapping between CNINode and Node objects // Returns nil if count and 1:1 mapping exists, else returns error -func VerifyCNINodeCount(manager Manager) error { +func VerifyCNINode(manager Manager) error { cniNodes, err := manager.GetCNINodeList() Expect(err).NotTo(HaveOccurred()) nodes, err := manager.GetNodeList() diff --git a/test/integration/cninode/cninode_suite_test.go b/test/integration/cninode/cninode_suite_test.go index cff568f1..83411fbb 100644 --- a/test/integration/cninode/cninode_suite_test.go +++ b/test/integration/cninode/cninode_suite_test.go @@ -38,13 +38,13 @@ var _ = BeforeSuite(func() { Expect(len(nodeList.Items)).To(BeNumerically(">", 1)) By("verify CNINode count") - err = node.VerifyCNINodeCount(frameWork.NodeManager) + err = node.VerifyCNINode(frameWork.NodeManager) Expect(err).ToNot(HaveOccurred()) }) // Verify CNINode count before and after test remains same var _ = AfterSuite(func() { By("verify CNINode count") - err := node.VerifyCNINodeCount(frameWork.NodeManager) + err := node.VerifyCNINode(frameWork.NodeManager) Expect(err).ToNot(HaveOccurred()) }) diff --git a/test/integration/cninode/cninode_test.go b/test/integration/cninode/cninode_test.go index ad23f5ac..c680bfd7 100644 --- a/test/integration/cninode/cninode_test.go +++ b/test/integration/cninode/cninode_test.go @@ -14,7 +14,7 @@ package cninode_test import ( - "time" + "context" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils" @@ -22,6 +22,7 @@ import ( testUtils "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/util/wait" ) var _ = Describe("[CANARY]CNINode test", func() { @@ -44,8 +45,7 @@ var _ = Describe("[CANARY]CNINode test", func() { By("restoring ASG minSize & maxSize after test") err := frameWork.AutoScalingManager.UpdateAutoScalingGroup(asgName, oldDesiredSize, oldMinSize, oldMaxSize) Expect(err).ToNot(HaveOccurred()) - // sleep to allow ASG to be updated - time.Sleep(testUtils.ResourceCreationTimeout) + Expect(WaitTillNodeSizeUpdated(int(oldDesiredSize))).Should(Succeed()) }) Context("when new node is added", func() { @@ -55,11 +55,8 @@ var _ = Describe("[CANARY]CNINode test", func() { By("adding new node") err := frameWork.AutoScalingManager.UpdateAutoScalingGroup(asgName, newSize, oldMinSize, newSize) Expect(err).ToNot(HaveOccurred()) - // sleep to allow ASG to be updated - time.Sleep(testUtils.ResourceCreationTimeout) - - err = node.VerifyCNINodeCount(frameWork.NodeManager) - Expect(err).ToNot(HaveOccurred()) + Expect(WaitTillNodeSizeUpdated(int(newSize))).Should(Succeed()) + Expect(node.VerifyCNINode(frameWork.NodeManager)).Should(Succeed()) }) }) Context("when existing node is removed", func() { @@ -69,11 +66,8 @@ var _ = Describe("[CANARY]CNINode test", func() { By("removing existing node") err := frameWork.AutoScalingManager.UpdateAutoScalingGroup(asgName, newSize, newSize, oldMaxSize) Expect(err).ToNot(HaveOccurred()) - // sleep to allow ASG to be updated - time.Sleep(testUtils.ResourceCreationTimeout) - err = node.VerifyCNINodeCount(frameWork.NodeManager) - Expect(err).ToNot(HaveOccurred()) - + Expect(WaitTillNodeSizeUpdated(int(newSize))).Should(Succeed()) + Expect(node.VerifyCNINode(frameWork.NodeManager)).Should(Succeed()) }) }) }) @@ -93,3 +87,19 @@ func ListNodesAndGetAutoScalingGroupName() string { Expect(ok).To(BeTrue()) return val } + +// Verifies (linux) node size is updated after ASG is updated +func WaitTillNodeSizeUpdated(desiredSize int) error { + err := wait.PollUntilContextTimeout(context.Background(), testUtils.PollIntervalShort, testUtils.ResourceCreationTimeout, true, + func(ctx context.Context) (bool, error) { + nodes, err := frameWork.NodeManager.GetNodesWithOS(config.OSLinux) // since we are only updating the linux ASG in the test + if err != nil { + return false, nil + } + if len(nodes.Items) != desiredSize { + return false, nil + } + return true, nil + }) + return err +} diff --git a/test/integration/perpodsg/perpodsg_suite_test.go b/test/integration/perpodsg/perpodsg_suite_test.go index 7198297a..584e6b55 100644 --- a/test/integration/perpodsg/perpodsg_suite_test.go +++ b/test/integration/perpodsg/perpodsg_suite_test.go @@ -54,7 +54,7 @@ var _ = BeforeSuite(func() { nodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, "linux", config.ResourceNamePodENI) - err = node.VerifyCNINodeCount(frameWork.NodeManager) + err = node.VerifyCNINode(frameWork.NodeManager) Expect(err).ToNot(HaveOccurred()) }) From 8b9330a720b9ec2edc3425ab36f9b2ed45dc8bb3 Mon Sep 17 00:00:00 2001 From: sushrk Date: Fri, 27 Sep 2024 23:17:43 +0000 Subject: [PATCH 3/4] updating log statements --- test/integration/cninode/cninode_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/cninode/cninode_test.go b/test/integration/cninode/cninode_test.go index c680bfd7..8173a348 100644 --- a/test/integration/cninode/cninode_test.go +++ b/test/integration/cninode/cninode_test.go @@ -42,7 +42,7 @@ var _ = Describe("[CANARY]CNINode test", func() { oldMaxSize = *asg[0].MaxSize }) AfterEach(func() { - By("restoring ASG minSize & maxSize after test") + By("restoring ASG desiredCapacity, minSize, maxSize after test") err := frameWork.AutoScalingManager.UpdateAutoScalingGroup(asgName, oldDesiredSize, oldMinSize, oldMaxSize) Expect(err).ToNot(HaveOccurred()) Expect(WaitTillNodeSizeUpdated(int(oldDesiredSize))).Should(Succeed()) @@ -90,6 +90,7 @@ func ListNodesAndGetAutoScalingGroupName() string { // Verifies (linux) node size is updated after ASG is updated func WaitTillNodeSizeUpdated(desiredSize int) error { + By("waiting till node list is updated") err := wait.PollUntilContextTimeout(context.Background(), testUtils.PollIntervalShort, testUtils.ResourceCreationTimeout, true, func(ctx context.Context) (bool, error) { nodes, err := frameWork.NodeManager.GetNodesWithOS(config.OSLinux) // since we are only updating the linux ASG in the test From bb84426ebf0333e5111262821e0590be8a07ee7d Mon Sep 17 00:00:00 2001 From: sushrk Date: Thu, 10 Oct 2024 08:18:22 +0000 Subject: [PATCH 4/4] add retry in VerifyCNINode --- test/framework/resource/k8s/node/wrapper.go | 28 ++++++++++++++------- test/framework/utils/poll.go | 1 + 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/test/framework/resource/k8s/node/wrapper.go b/test/framework/resource/k8s/node/wrapper.go index fc54272d..a486d63a 100644 --- a/test/framework/resource/k8s/node/wrapper.go +++ b/test/framework/resource/k8s/node/wrapper.go @@ -49,20 +49,30 @@ func GetNodeAndWaitTillCapacityPresent(manager Manager, os string, expectedResou // VerifyCNINode checks if the number of CNINodes is equal to number of nodes in the cluster, and verifies 1:1 mapping between CNINode and Node objects // Returns nil if count and 1:1 mapping exists, else returns error func VerifyCNINode(manager Manager) error { - cniNodes, err := manager.GetCNINodeList() - Expect(err).NotTo(HaveOccurred()) - nodes, err := manager.GetNodeList() - Expect(err).NotTo(HaveOccurred()) + var cniNodeList *cninode.CNINodeList + var nodeList *v1.NodeList + var err error By("checking number of CNINodes match number of nodes in the cluster") - isEqual := len(nodes.Items) == len(cniNodes.Items) - if !isEqual { + err = wait.PollUntilContextTimeout(context.Background(), utils.PollIntervalShort, utils.PollTimeout, true, + func(ctx context.Context) (bool, error) { + if cniNodeList, err = manager.GetCNINodeList(); err != nil { + return false, nil + } + if nodeList, err = manager.GetNodeList(); err != nil { + return false, nil + } + if len(nodeList.Items) != len(cniNodeList.Items) { + return false, nil + } + return true, nil + }) + if err != nil { return fmt.Errorf("number of CNINodes does not match number of nodes in the cluster") } - By("checking CNINode list matches node list") nameMatched := true - for _, node := range nodes.Items { - if !lo.ContainsBy(cniNodes.Items, func(cniNode cninode.CNINode) bool { + for _, node := range nodeList.Items { + if !lo.ContainsBy(cniNodeList.Items, func(cniNode cninode.CNINode) bool { return cniNode.Name == node.Name }) { nameMatched = false diff --git a/test/framework/utils/poll.go b/test/framework/utils/poll.go index cb343b16..285d1207 100644 --- a/test/framework/utils/poll.go +++ b/test/framework/utils/poll.go @@ -19,6 +19,7 @@ const ( PollIntervalShort = 2 * time.Second PollIntervalMedium = 10 * time.Second PollIntervalLong = 20 * time.Second + PollTimeout = 30 * time.Second // ResourceCreationTimeout is the number of seconds till the controller waits // for the resource creation to complete ResourceCreationTimeout = 120 * time.Second