From bd490e7b736b1784bfe4f4215168af67e74de6eb Mon Sep 17 00:00:00 2001 From: Sushmitha Ravikumar Date: Fri, 22 Jul 2022 17:06:29 -0700 Subject: [PATCH] fix integration test failures and cleanup --- test/framework/resources/aws/services/iam.go | 8 +++ test/integration/ipamd/eni_ip_leak_test.go | 8 --- test/integration/ipamd/eni_tag_test.go | 6 --- test/integration/ipamd/introspection_test.go | 9 ---- test/integration/ipamd/ipamd_suite_test.go | 19 +++++++ test/integration/ipamd/metrics_test.go | 12 ----- .../ipamd/warm_target_test_PD_enabled.go | 1 - .../metrics-helper/metric_helper_test.go | 12 ----- .../metrics_helper_suite_test.go | 51 ++++++++----------- 9 files changed, 49 insertions(+), 77 deletions(-) diff --git a/test/framework/resources/aws/services/iam.go b/test/framework/resources/aws/services/iam.go index 2a990faccc7..d7d8d7e14e8 100644 --- a/test/framework/resources/aws/services/iam.go +++ b/test/framework/resources/aws/services/iam.go @@ -37,6 +37,7 @@ type IAM interface { CreatePolicy(policyName string, policyDocument string) (*iam.CreatePolicyOutput, error) DeletePolicy(policyARN string) error GetInstanceProfile(instanceProfileName string) (*iam.GetInstanceProfileOutput, error) + ListPolicies(scope string) (*iam.ListPoliciesOutput, error) } type defaultIAM struct { @@ -84,6 +85,13 @@ func (d *defaultIAM) GetInstanceProfile(instanceProfileName string) (*iam.GetIns return d.IAMAPI.GetInstanceProfile(getInstanceProfileInput) } +func (d *defaultIAM) ListPolicies(scope string) (*iam.ListPoliciesOutput, error) { + listPolicyInput := &iam.ListPoliciesInput{ + Scope: aws.String(scope), + } + return d.IAMAPI.ListPolicies(listPolicyInput) +} + func NewIAM(session *session.Session) IAM { return &defaultIAM{ IAMAPI: iam.New(session), diff --git a/test/integration/ipamd/eni_ip_leak_test.go b/test/integration/ipamd/eni_ip_leak_test.go index 4446e0fc487..1c5947de0a7 100644 --- a/test/integration/ipamd/eni_ip_leak_test.go +++ b/test/integration/ipamd/eni_ip_leak_test.go @@ -22,11 +22,6 @@ var numOfNodes int var _ = Describe("[CANARY] ENI/IP Leak Test", func() { Context("ENI/IP Released on Pod Deletion", func() { - BeforeEach(func() { - By("creating test namespace") - f.K8sResourceManagers.NamespaceManager(). - CreateNamespace(utils.DefaultTestNamespace) - }) It("Verify that on Pod Deletion, ENI/IP State is restored", func() { // Set the WARM_ENI_TARGET to 0 to prevent all pods being scheduled on secondary ENI @@ -72,9 +67,6 @@ var _ = Describe("[CANARY] ENI/IP Leak Test", func() { }) AfterEach(func() { - By("deleting test namespace") - f.K8sResourceManagers.NamespaceManager(). - DeleteAndWaitTillNamespaceDeleted(utils.DefaultTestNamespace) By("Restoring WARM ENI Target value") k8sUtils.RemoveVarFromDaemonSetAndWaitTillUpdated(f, "aws-node", "kube-system", diff --git a/test/integration/ipamd/eni_tag_test.go b/test/integration/ipamd/eni_tag_test.go index c8ab5a7e834..3bfa75fb904 100644 --- a/test/integration/ipamd/eni_tag_test.go +++ b/test/integration/ipamd/eni_tag_test.go @@ -39,9 +39,6 @@ var _ = Describe("test tags are created on Secondary ENI", func() { // sets the desired environment variables and gets the list of new ENIs created after setting // the environment variables JustBeforeEach(func() { - By("creating test namespace") - f.K8sResourceManagers.NamespaceManager(). - CreateNamespace(utils.DefaultTestNamespace) // To re-initialize for each test case newENIs = []string{} @@ -84,9 +81,6 @@ var _ = Describe("test tags are created on Secondary ENI", func() { }) JustAfterEach(func() { - By("deleting test namespace") - f.K8sResourceManagers.NamespaceManager(). - DeleteAndWaitTillNamespaceDeleted(utils.DefaultTestNamespace) envVarToRemove := map[string]struct{}{} for key, _ := range environmentVariables { diff --git a/test/integration/ipamd/introspection_test.go b/test/integration/ipamd/introspection_test.go index d31375a7be2..178ee6b8996 100644 --- a/test/integration/ipamd/introspection_test.go +++ b/test/integration/ipamd/introspection_test.go @@ -34,9 +34,6 @@ var _ = Describe("test Environment Variables for IPAMD Introspection ", func() { var curlJob *v1.Job JustBeforeEach(func() { - By("creating test namespace") - f.K8sResourceManagers.NamespaceManager(). - CreateNamespace(utils.DefaultTestNamespace) // Initially the host networking job pod should succeed curlContainer = manifest.NewCurlContainer(). @@ -67,12 +64,6 @@ var _ = Describe("test Environment Variables for IPAMD Introspection ", func() { Expect(err).ToNot(HaveOccurred()) }) - JustAfterEach(func() { - By("deleting test namespace") - f.K8sResourceManagers.NamespaceManager(). - DeleteAndWaitTillNamespaceDeleted(utils.DefaultTestNamespace) - }) - Context("when disabling introspection by setting DISABLE_INTROSPECTION to true", func() { It("introspection should not work anymore", func() { diff --git a/test/integration/ipamd/ipamd_suite_test.go b/test/integration/ipamd/ipamd_suite_test.go index 441302f4d60..c4b25b9e57d 100644 --- a/test/integration/ipamd/ipamd_suite_test.go +++ b/test/integration/ipamd/ipamd_suite_test.go @@ -15,9 +15,11 @@ package ipamd import ( "testing" + "time" "github.com/aws/amazon-vpc-cni-k8s/test/framework" k8sUtils "github.com/aws/amazon-vpc-cni-k8s/test/framework/resources/k8s/utils" + "github.com/aws/amazon-vpc-cni-k8s/test/framework/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -30,6 +32,10 @@ func TestIPAMD(t *testing.T) { var _ = BeforeSuite(func() { f = framework.New(framework.GlobalOptions) + By("creating test namespace") + f.K8sResourceManagers.NamespaceManager(). + CreateNamespace(utils.DefaultTestNamespace) + nodeList, err := f.K8sResourceManagers.NodeManager().GetNodes(f.Options.NgNameLabelKey, f.Options.NgNameLabelVal) Expect(err).ToNot(HaveOccurred()) @@ -43,4 +49,17 @@ var _ = BeforeSuite(func() { instanceID := k8sUtils.GetInstanceIDFromNode(primaryNode) primaryInstance, err = f.CloudServices.EC2().DescribeInstance(instanceID) Expect(err).ToNot(HaveOccurred()) + + // Remove WARM_ENI_TARGET, WARM_IP_TARGET, MINIMUM_IP_TARGET and WARM_PREFIX_TARGET before running IPAMD tests + k8sUtils.RemoveVarFromDaemonSetAndWaitTillUpdated(f, "aws-node", "kube-system", + "aws-node", map[string]struct{}{"WARM_ENI_TARGET": {}, "WARM_IP_TARGET": {}, "MINIMUM_IP_TARGET": {}, "WARM_PREFIX_TARGET": {}}) + + // Allow reconciler to free up ENIs if any + time.Sleep(utils.PollIntervalLong) +}) + +var _ = AfterSuite(func() { + By("deleting test namespace") + f.K8sResourceManagers.NamespaceManager(). + DeleteAndWaitTillNamespaceDeleted(utils.DefaultTestNamespace) }) diff --git a/test/integration/ipamd/metrics_test.go b/test/integration/ipamd/metrics_test.go index facc6278efd..d9673749560 100644 --- a/test/integration/ipamd/metrics_test.go +++ b/test/integration/ipamd/metrics_test.go @@ -32,18 +32,6 @@ var _ = Describe("test IPAMD metric environment variable", func() { // Job's output determines if the API is reachable or not var curlJob *v1.Job - JustBeforeEach(func() { - By("creating test namespace") - f.K8sResourceManagers.NamespaceManager(). - CreateNamespace(utils.DefaultTestNamespace) - }) - - JustAfterEach(func() { - By("deleting test namespace") - f.K8sResourceManagers.NamespaceManager(). - DeleteAndWaitTillNamespaceDeleted(utils.DefaultTestNamespace) - }) - Context("when metrics is disabled", func() { metricAddr := "127.0.0.1:61678/metrics" It("should not be accessible anymore", func() { diff --git a/test/integration/ipamd/warm_target_test_PD_enabled.go b/test/integration/ipamd/warm_target_test_PD_enabled.go index e115c588a7f..7618ceac412 100644 --- a/test/integration/ipamd/warm_target_test_PD_enabled.go +++ b/test/integration/ipamd/warm_target_test_PD_enabled.go @@ -73,7 +73,6 @@ var _ = Describe("test warm target variables", func() { *primaryInstance.PrivateDnsName, pod.Status.PodIP, pod.Spec.NodeName)) if pod.Spec.NodeName == *primaryInstance.PrivateDnsName { assigned++ - break } } diff --git a/test/integration/metrics-helper/metric_helper_test.go b/test/integration/metrics-helper/metric_helper_test.go index b84c45d6a22..252d151dde9 100644 --- a/test/integration/metrics-helper/metric_helper_test.go +++ b/test/integration/metrics-helper/metric_helper_test.go @@ -28,18 +28,6 @@ import ( var _ = Describe("test cni-metrics-helper publishes metrics", func() { - JustBeforeEach(func() { - By("creating test namespace") - f.K8sResourceManagers.NamespaceManager(). - CreateNamespace(utils.DefaultTestNamespace) - }) - - JustAfterEach(func() { - By("deleting test namespace") - f.K8sResourceManagers.NamespaceManager(). - DeleteAndWaitTillNamespaceDeleted(utils.DefaultTestNamespace) - }) - Context("when a metric is updated", func() { It("the updated metric is published to CW", func() { diff --git a/test/integration/metrics-helper/metrics_helper_suite_test.go b/test/integration/metrics-helper/metrics_helper_suite_test.go index e36d1218e10..cefb8f37a54 100644 --- a/test/integration/metrics-helper/metrics_helper_suite_test.go +++ b/test/integration/metrics-helper/metrics_helper_suite_test.go @@ -14,13 +14,11 @@ package metrics_helper import ( - "encoding/json" "flag" "strings" "testing" "github.com/aws/amazon-vpc-cni-k8s/test/framework" - "github.com/aws/amazon-vpc-cni-k8s/test/framework/resources/aws/services" k8sUtil "github.com/aws/amazon-vpc-cni-k8s/test/framework/resources/k8s/utils" "github.com/aws/amazon-vpc-cni-k8s/test/framework/utils" @@ -72,28 +70,11 @@ func TestCNIMetricsHelper(t *testing.T) { var _ = BeforeSuite(func() { f = framework.New(framework.GlobalOptions) - // Create a new policy with PutMetric Permission - policy := services.PolicyDocument{ - Version: "2012-10-17", - Statement: []services.StatementEntry{ - { - Effect: "Allow", - Action: []string{"cloudwatch:PutMetricData"}, - Resource: "*", - }, - }, - } - - b, err := json.Marshal(policy) - Expect(err).ToNot(HaveOccurred()) + By("creating test namespace") + f.K8sResourceManagers.NamespaceManager(). + CreateNamespace(utils.DefaultTestNamespace) - By("creating the CNIMetricsHelperPolicy policy") - createPolicyOutput, err := f.CloudServices.IAM(). - CreatePolicy("CNIMetricsHelperPolicy", string(b)) - Expect(err).ToNot(HaveOccurred()) - policyARN = *createPolicyOutput.Policy.Arn - - By("getting the node instance profile") + By("getting the node list") nodeList, err := f.K8sResourceManagers.NodeManager().GetAllNodes() Expect(err).ToNot(HaveOccurred()) Expect(len(nodeList.Items)).To(BeNumerically(">", 0)) @@ -133,9 +114,21 @@ var _ = BeforeSuite(func() { instanceProfileOutput, err := f.CloudServices.IAM().GetInstanceProfile(instanceProfileRoleName) Expect(err).ToNot(HaveOccurred()) - By("attaching policy to the node IAM role") ngRoleName = *instanceProfileOutput.InstanceProfile.Roles[0].RoleName - By("attaching the node instance role") + By("attaching CNIMetricsHelperPolicy to the node IAM role") + + // We should ideally use the PathPrefix argument to list the policy, but this is returning an empty list. So workaround by listing local policies & filter + // SO issue: https://stackoverflow.com/questions/66287626/aws-cli-list-policies-to-find-a-policy-with-a-specific-name + policyList, err := f.CloudServices.IAM().ListPolicies("Local") + Expect(err).ToNot((HaveOccurred())) + + for _, item := range policyList.Policies { + if strings.Contains(*item.PolicyName, "CNIMetricsHelperPolicy") { + policyARN = *item.Arn + break + } + } + err = f.CloudServices.IAM().AttachRolePolicy(policyARN, ngRoleName) Expect(err).ToNot(HaveOccurred()) @@ -156,11 +149,11 @@ var _ = AfterSuite(func() { err = f.CloudServices.IAM().DetachRolePolicy(policyARN, ngRoleName) Expect(err).ToNot(HaveOccurred()) - By("deleting the CNIMetricsHelperPolicy policy") - err = f.CloudServices.IAM().DeletePolicy(policyARN) - Expect(err).ToNot(HaveOccurred()) - By("uninstalling cni-metrics-helper using helm") err := f.InstallationManager.UnInstallCNIMetricsHelper() Expect(err).ToNot(HaveOccurred()) + + By("deleting test namespace") + f.K8sResourceManagers.NamespaceManager(). + DeleteAndWaitTillNamespaceDeleted(utils.DefaultTestNamespace) })