From 50b536e6823c3a5ebef385407f0edf5ee28d6d32 Mon Sep 17 00:00:00 2001 From: Hao Zhou Date: Tue, 20 Feb 2024 00:59:34 +0000 Subject: [PATCH] rename windows flags --- pkg/config/loader.go | 9 ++ pkg/config/type.go | 3 + pkg/provider/prefix/provider_test.go | 78 +++++++------ test/integration/windows/windows_test.go | 134 +++++++++++++++++++++++ 4 files changed, 193 insertions(+), 31 deletions(-) diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 1e9c7608..90d1b61d 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -92,8 +92,17 @@ func ParseWinPDTargets(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTa } warmIPTargetStr, foundWarmIP := vpcCniConfigMap.Data[WarmIPTarget] + if !foundWarmIP { + warmIPTargetStr, foundWarmIP = vpcCniConfigMap.Data[WinWarmIPTarget] + } minIPTargetStr, foundMinIP := vpcCniConfigMap.Data[MinimumIPTarget] + if !foundMinIP { + minIPTargetStr, foundMinIP = vpcCniConfigMap.Data[WinMinimumIPTarget] + } warmPrefixTargetStr, foundWarmPrefix := vpcCniConfigMap.Data[WarmPrefixTarget] + if !foundWarmPrefix { + warmPrefixTargetStr, foundWarmPrefix = vpcCniConfigMap.Data[WinWarmPrefixTarget] + } // If no configuration is found, return 0 if !foundWarmIP && !foundMinIP && !foundWarmPrefix { diff --git a/pkg/config/type.go b/pkg/config/type.go index f46f2621..550e8bc3 100644 --- a/pkg/config/type.go +++ b/pkg/config/type.go @@ -76,6 +76,9 @@ const ( WarmPrefixTarget = "warm-prefix-target" WarmIPTarget = "warm-ip-target" MinimumIPTarget = "minimum-ip-target" + WinWarmPrefixTarget = "windows-warm-prefix-target" + WinWarmIPTarget = "windows-warm-ip-target" + WinMinimumIPTarget = "windows-minimum-ip-target" // Since LeaderElectionNamespace and VpcCniConfigMapName may be different in the future KubeSystemNamespace = "kube-system" VpcCNIDaemonSetName = "aws-node" diff --git a/pkg/provider/prefix/provider_test.go b/pkg/provider/prefix/provider_test.go index 70410aff..3daea497 100644 --- a/pkg/provider/prefix/provider_test.go +++ b/pkg/provider/prefix/provider_test.go @@ -22,9 +22,9 @@ import ( mock_ec2 "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2" mock_condition "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/condition" mock_k8s "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s" - "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/pool" - "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni" - "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/worker" + mock_pool "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/pool" + mock_eni "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/provider/ip/eni" + mock_worker "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/worker" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/api" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool" @@ -68,6 +68,16 @@ var ( }, } + vpcCNIConfigWindows = &v1.ConfigMap{ + Data: map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "true", + config.WinWarmIPTarget: strconv.Itoa(config.IPv4PDDefaultWarmIPTargetSize), + config.WinMinimumIPTarget: strconv.Itoa(config.IPv4PDDefaultMinIPTargetSize), + config.WinWarmPrefixTarget: strconv.Itoa(config.IPv4PDDefaultWarmPrefixTargetSize), + }, + } + node = &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: nodeName, @@ -386,23 +396,25 @@ func TestIPv4PrefixProvider_UpdateResourceCapacity_FromFromIPToPD(t *testing.T) instanceProviderAndPool: map[string]*ResourceProviderAndPool{}, log: zap.New(zap.UseDevMode(true)).WithName("prefix provider"), conditions: mockConditions} - mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(vpcCNIConfig, nil) - mockPool := mock_pool.NewMockPool(ctrl) - mockManager := mock_eni.NewMockENIManager(ctrl) - prefixProvider.putInstanceProviderAndPool(nodeName, mockPool, mockManager, nodeCapacity, true) - mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(true) + for _, c := range []*v1.ConfigMap{vpcCNIConfig, vpcCNIConfigWindows} { + mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(c, nil) + mockPool := mock_pool.NewMockPool(ctrl) + mockManager := mock_eni.NewMockENIManager(ctrl) + prefixProvider.putInstanceProviderAndPool(nodeName, mockPool, mockManager, nodeCapacity, true) + mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(true) - job := &worker.WarmPoolJob{Operations: worker.OperationCreate} - mockPool.EXPECT().SetToActive(pdWarmPoolConfig).Return(job) - mockWorker.EXPECT().SubmitJob(job) + job := &worker.WarmPoolJob{Operations: worker.OperationCreate} + mockPool.EXPECT().SetToActive(pdWarmPoolConfig).Return(job) + mockWorker.EXPECT().SubmitJob(job) - mockInstance.EXPECT().Name().Return(nodeName).Times(2) - mockInstance.EXPECT().Type().Return(instanceType) - mockInstance.EXPECT().Os().Return(config.OSWindows) - mockK8sWrapper.EXPECT().AdvertiseCapacityIfNotSet(nodeName, config.ResourceNameIPAddress, 224).Return(nil) + mockInstance.EXPECT().Name().Return(nodeName).Times(2) + mockInstance.EXPECT().Type().Return(instanceType) + mockInstance.EXPECT().Os().Return(config.OSWindows) + mockK8sWrapper.EXPECT().AdvertiseCapacityIfNotSet(nodeName, config.ResourceNameIPAddress, 224).Return(nil) - err := prefixProvider.UpdateResourceCapacity(mockInstance) - assert.NoError(t, err) + err := prefixProvider.UpdateResourceCapacity(mockInstance) + assert.NoError(t, err) + } } // TestIPv4PrefixProvider_UpdateResourceCapacity_FromFromPDToIP tests the warm pool is drained when PD is disabled @@ -449,21 +461,23 @@ func TestIPv4PrefixProvider_UpdateResourceCapacity_FromPDToPD(t *testing.T) { mockPool := mock_pool.NewMockPool(ctrl) mockManager := mock_eni.NewMockENIManager(ctrl) prefixProvider.putInstanceProviderAndPool(nodeName, mockPool, mockManager, nodeCapacity, true) - mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(true) - mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(vpcCNIConfig, nil) + for _, c := range []*v1.ConfigMap{vpcCNIConfig, vpcCNIConfigWindows} { + mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(true) + mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(c, nil) - job := &worker.WarmPoolJob{Operations: worker.OperationCreate} - mockPool.EXPECT().SetToActive(pdWarmPoolConfig).Return(job) - mockWorker.EXPECT().SubmitJob(job) + job := &worker.WarmPoolJob{Operations: worker.OperationCreate} + mockPool.EXPECT().SetToActive(pdWarmPoolConfig).Return(job) + mockWorker.EXPECT().SubmitJob(job) - mockInstance.EXPECT().Name().Return(nodeName).Times(2) - mockInstance.EXPECT().Type().Return(instanceType) - mockInstance.EXPECT().Os().Return(config.OSWindows) - mockK8sWrapper.EXPECT().AdvertiseCapacityIfNotSet(nodeName, config.ResourceNameIPAddress, 224).Return(nil) + mockInstance.EXPECT().Name().Return(nodeName).Times(2) + mockInstance.EXPECT().Type().Return(instanceType) + mockInstance.EXPECT().Os().Return(config.OSWindows) + mockK8sWrapper.EXPECT().AdvertiseCapacityIfNotSet(nodeName, config.ResourceNameIPAddress, 224).Return(nil) - err := prefixProvider.UpdateResourceCapacity(mockInstance) - assert.NoError(t, err) + err := prefixProvider.UpdateResourceCapacity(mockInstance) + assert.NoError(t, err) + } } // TestIPv4PrefixProvider_UpdateResourceCapacity_FromIPToIP tests the resource capacity is not updated when secondary IP mode stays enabled @@ -539,10 +553,12 @@ func TestGetPDWarmPoolConfig(t *testing.T) { instanceProviderAndPool: map[string]*ResourceProviderAndPool{}, log: zap.New(zap.UseDevMode(true)).WithName("prefix provider"), conditions: mockConditions} - mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(vpcCNIConfig, nil) + for _, c := range []*v1.ConfigMap{vpcCNIConfig, vpcCNIConfigWindows} { + mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(c, nil) - config := prefixProvider.getPDWarmPoolConfig() - assert.Equal(t, pdWarmPoolConfig, config) + config := prefixProvider.getPDWarmPoolConfig() + assert.Equal(t, pdWarmPoolConfig, config) + } } // TestIsInstanceSupported tests that if the instance type is nitro, return true diff --git a/test/integration/windows/windows_test.go b/test/integration/windows/windows_test.go index 343d729b..24962b4a 100644 --- a/test/integration/windows/windows_test.go +++ b/test/integration/windows/windows_test.go @@ -285,6 +285,7 @@ var _ = Describe("Windows Integration Test", func() { }) }) + // TODO: remove this context when VPC CNI also updates the flag name to windows prefixed. Context("When warm-prefix-target is set to 2", Label("warm-prefix-target"), func() { BeforeEach(func() { data = map[string]string{ @@ -316,6 +317,7 @@ var _ = Describe("Windows Integration Test", func() { }) }) + // TODO: remove this context when VPC CNI also updates the flag name to windows prefixed. Context("When warm-ip-target is set to 15", Label("warm-ip-target"), func() { BeforeEach(func() { data = map[string]string{ @@ -362,6 +364,7 @@ var _ = Describe("Windows Integration Test", func() { }) }) + // TODO: remove this context when VPC CNI also updates the flag name to windows prefixed. Context("When minimum-ip-target is set to 20", Label("minimum-ip-target"), func() { BeforeEach(func() { data = map[string]string{ @@ -415,6 +418,137 @@ var _ = Describe("Windows Integration Test", func() { }) }) + Context("When windows-warm-prefix-target is set to 2", Label("windows-warm-prefix-target"), func() { + BeforeEach(func() { + data = map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "true", + config.WinWarmPrefixTarget: "2"} + + }) + + It("two prefixes should be assigned", func() { + // allow some time for previous test pod to cool down + time.Sleep(bufferForCoolDown) + _, prefixesBefore, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(prefixesBefore)).To(Equal(2)) + + By("creating pod and waiting for ready should have 1 new prefix assigned") + // verify if ip assigned is coming from a prefix + createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + verify.WindowsPodHaveIPv4AddressFromPrefixes(createdPod, prefixesBefore) + + // number of prefixes should increase by 1 since need 1 more prefix to fulfill warm-prefix-target of 2 + _, prefixesAfter, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(prefixesAfter) - len(prefixesBefore)).To(Equal(1)) + + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Context("When windows-warm-ip-target is set to 15", Label("windows-warm-ip-target"), func() { + BeforeEach(func() { + data = map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "true", + config.WinWarmIPTarget: "15"} + }) + It("should assign new prefix when 2nd pod is launched", func() { + // allow some time for previous test pod to cool down + time.Sleep(bufferForCoolDown) + // before running any pod, should have 1 prefix assigned + privateIPsBefore, prefixesBefore, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(prefixesBefore)).To(Equal(1)) + + By("creating 1 pod and waiting for ready should not create new prefix") + // verify if ip assigned is coming from a prefix + createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + + _, prefixesAfterPod1, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(prefixesAfterPod1)).To(Equal(len(prefixesBefore))) + verify.WindowsPodHaveIPv4AddressFromPrefixes(createdPod, prefixesAfterPod1) + + // launch 2nd pod to trigger a new prefix to be assigned since warm-ip-target=15 + By("creating 2nd pod and waiting for ready should have 1 more prefix assigned") + createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod2, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + verify.WindowsPodHaveResourceLimits(createdPod, true) + + privateIPsAfter, prefixesAfterPod2, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + // 1 more prefix should be created to fulfill warm-ip-target=15 + Expect(len(prefixesAfterPod2) - len(prefixesAfterPod1)).To(Equal(1)) + // number of secondary ips should not change + Expect(len(privateIPsBefore)).To(Equal(len(privateIPsAfter))) + verify.WindowsPodHaveIPv4AddressFromPrefixes(createdPod, prefixesAfterPod2) + + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod) + Expect(err).ToNot(HaveOccurred()) + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod2) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Context("When windows-minimum-ip-target is set to 20", Label("windows-minimum-ip-target"), func() { + BeforeEach(func() { + data = map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "true", + config.WinMinimumIPTarget: "20"} + }) + It("should have 2 prefixes to satisfy windows-minimum-ip-target when no pods running", func() { + By("adding labels to selected nodes for testing") + node := windowsNodeList.Items[0] + err = frameWork.NodeManager.AddLabels([]v1.Node{node}, map[string]string{podLabelKey: podLabelVal}) + Expect(err).ToNot(HaveOccurred()) + + // allow some time for previous test pod to cool down + time.Sleep(bufferForCoolDown) + // before running any pod, should have 2 prefixes assigned + instanceID = manager.GetNodeInstanceID(&node) + privateIPsBefore, prefixesBefore, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(prefixesBefore)).To(Equal(2)) + + By("creating 33 pods and waiting for ready should have 3 prefixes attached") + deployment := manifest.NewWindowsDeploymentBuilder(). + Replicas(33). + Container(manifest.NewWindowsContainerBuilder().Build()). + PodLabel(podLabelKey, podLabelVal). + NodeSelector(map[string]string{"kubernetes.io/os": "windows", podLabelKey: podLabelVal}). + Build() + _, err = frameWork.DeploymentManager.CreateAndWaitUntilDeploymentReady(ctx, deployment) + Expect(err).ToNot(HaveOccurred()) + + _, prefixesAfterDeployment, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(prefixesAfterDeployment)).To(Equal(3)) + + By("deleting 33 pods should still have 2 prefixes attached") + err = frameWork.DeploymentManager.DeleteAndWaitUntilDeploymentDeleted(ctx, deployment) + Expect(err).ToNot(HaveOccurred()) + + // allow some time for previous test pods to cool down since deletion of deployment doesn't wait for pods to terminate + time.Sleep(utils.WindowsPodsDeletionTimeout) + privateIPsAfter, prefixesAfterDelete, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(prefixesAfterDelete)).To(Equal(2)) + // number of secondary ips should not change + Expect(len(privateIPsBefore)).To(Equal(len(privateIPsAfter))) + + By("removing labels on selected nodes for testing") + err = frameWork.NodeManager.RemoveLabels([]v1.Node{node}, map[string]string{podLabelKey: podLabelVal}) + Expect(err).ToNot(HaveOccurred()) + }) + }) + Context("[CANARY] When enable-windows-prefix-delegation is toggled to false", func() { BeforeEach(func() { data = map[string]string{