Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rename windows flags #371

Merged
merged 1 commit into from
Feb 20, 2024
Merged

rename windows flags #371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pkg/config/loader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,17 @@ func ParseWinPDTargets(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTa
}

warmIPTargetStr, foundWarmIP := vpcCniConfigMap.Data[WarmIPTarget]
if !foundWarmIP {
jdn5126 marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down
11 changes: 8 additions & 3 deletions pkg/config/type.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,14 @@ const (
VpcCniConfigMapName = "amazon-vpc-cni"
EnableWindowsIPAMKey = "enable-windows-ipam"
EnableWindowsPrefixDelegationKey = "enable-windows-prefix-delegation"
WarmPrefixTarget = "warm-prefix-target"
WarmIPTarget = "warm-ip-target"
MinimumIPTarget = "minimum-ip-target"
// TODO: we will deprecate the confusing naming of Windows flags eventually
WarmPrefixTarget = "warm-prefix-target"
WarmIPTarget = "warm-ip-target"
MinimumIPTarget = "minimum-ip-target"
// these windows prefixed flags will be used for Windows support only eventully
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"
Expand Down
78 changes: 47 additions & 31 deletions pkg/provider/prefix/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
134 changes: 134 additions & 0 deletions test/integration/windows/windows_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
Loading