Skip to content

Commit

Permalink
rename windows flags
Browse files Browse the repository at this point in the history
  • Loading branch information
@haouc
haouc committed Feb 20, 2024
1 parent 18352ea commit 50b536e
Show file tree
Hide file tree
Showing 4 changed files with 193 additions and 31 deletions.
9 changes: 9 additions & 0 deletions pkg/config/loader.go
View file
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 {
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
3 changes: 3 additions & 0 deletions pkg/config/type.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
78 changes: 47 additions & 31 deletions pkg/provider/prefix/provider_test.go
View file
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
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

0 comments on commit 50b536e

Please sign in to comment.