Skip to content

Commit

Permalink
update unit tests and integration tests for CNINode (#278)
Browse files Browse the repository at this point in the history
  • Loading branch information
haouc authored Jul 11, 2023
1 parent 2d502cf commit 97695bd
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 38 deletions.
54 changes: 54 additions & 0 deletions pkg/node/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -898,3 +898,57 @@ func Test_GetEniConfigName(t *testing.T) {
})
}
}

func Test_TrunkEnabledInCNINode(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

dataStoreWithUnManagedNode := map[string]node.Node{v1Node.Name: unManagedNode}

mock := NewMock(ctrl, dataStoreWithUnManagedNode)

testCases := []struct {
features []rcV1alpha1.Feature
managed bool
msg string
}{
{
features: []rcV1alpha1.Feature{},
managed: false,
msg: "no feature is added and node is not managed",
},
{
features: []rcV1alpha1.Feature{
{
Name: rcV1alpha1.SecurityGroupsForPods,
Value: "",
},
},
managed: true,
msg: "no SGP feature is added and node is not managed",
},
{
features: []rcV1alpha1.Feature{
{
Name: rcV1alpha1.CustomNetworking,
Value: "default",
},
},
managed: false,
msg: "SGP feature is added and node is managed",
},
}

for _, test := range testCases {
t.Run(test.msg, func(t *testing.T) {
mock.MockK8sAPI.EXPECT().GetCNINode(types.NamespacedName{Name: v1Node.Name}).Return(&rcV1alpha1.CNINode{
Spec: rcV1alpha1.CNINodeSpec{
Features: test.features,
},
}, nil).Times(1)
managed, err := mock.Manager.trunkEnabledInCNINode(v1Node)
assert.NoError(t, err)
assert.Equal(t, test.managed, managed)
})
}
}
2 changes: 2 additions & 0 deletions test/framework/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package framework

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"
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"
Expand Down Expand Up @@ -72,6 +73,7 @@ func New(options Options) *Framework {
clientgoscheme.AddToScheme(k8sSchema)
sgp.AddToScheme(k8sSchema)
eniConfig.AddToScheme(k8sSchema)
cninode.AddToScheme(k8sSchema)

stopChan := ctrl.SetupSignalHandler()
cache, err := cache.New(config, cache.Options{Scheme: k8sSchema})
Expand Down
22 changes: 22 additions & 0 deletions test/framework/resource/k8s/node/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package node
import (
"context"

cninode "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1alpha1"
"github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/utils"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"
Expand All @@ -28,6 +29,9 @@ type Manager interface {
AddLabels(nodeList []v1.Node, label map[string]string) error
RemoveLabels(nodeList []v1.Node, label map[string]string) error
GetNode(node *v1.Node) (*v1.Node, error)
GetNodeList() (*v1.NodeList, error)
GetCNINode(node *v1.Node) (*cninode.CNINode, error)
GetCNINodeList() (*cninode.CNINodeList, error)
}

type defaultManager struct {
Expand Down Expand Up @@ -95,3 +99,21 @@ func (d *defaultManager) GetNode(node *v1.Node) (*v1.Node, error) {
err := d.k8sClient.Get(context.TODO(), utils.NamespacedName(node), observedNode)
return observedNode, err
}

func (d *defaultManager) GetCNINode(node *v1.Node) (*cninode.CNINode, error) {
cniNode := &cninode.CNINode{}
err := d.k8sClient.Get(context.TODO(), types.NamespacedName{Name: node.Name}, cniNode)
return cniNode, err
}

func (d *defaultManager) GetCNINodeList() (*cninode.CNINodeList, error) {
list := &cninode.CNINodeList{}
err := d.k8sClient.List(context.TODO(), list)
return list, err
}

func (d *defaultManager) GetNodeList() (*v1.NodeList, error) {
list := &v1.NodeList{}
err := d.k8sClient.List(context.TODO(), list)
return list, err
}
112 changes: 74 additions & 38 deletions test/integration/perpodsg/perpodsg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package perpodsg_test
import (
"time"

cninode "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1alpha1"
"github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1beta1"
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch"
Expand All @@ -26,6 +27,7 @@ import (
podWrapper "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/resource/k8s/pod"
sgpWrapper "github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/resource/k8s/sgp"
"github.com/aws/amazon-vpc-resource-controller-k8s/test/framework/utils"
"github.com/samber/lo"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -35,6 +37,29 @@ 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
Expand Down Expand Up @@ -427,44 +452,55 @@ var _ = Describe("Branch ENI Pods", func() {
It("pod should not run when un-managed and run when managed", func() {
node := targetedNodes[0]

By("verifying node has trunk ENI label present")
// This label is added by IPAM-D
_, found := node.Labels[config.HasTrunkAttachedLabel]
Expect(found).To(BeTrue())

// This should never happens as once the trunk is attached,
// this label will not be removed again. This is for testing
// purposes to make a managed node an un-managed node
By("removing the has-trunk-attached label from the node")
err = frameWork.NodeManager.RemoveLabels(targetedNodes,
map[string]string{config.HasTrunkAttachedLabel: "true"})

firstPod := podTemplate.DeepCopy()
By("creating a Pod on the un-managed node and verifying it fails")
_, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, firstPod, utils.ResourceCreationTimeout)
Expect(err).To(HaveOccurred())

By("deleting the pod")
err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, firstPod)
Expect(err).ToNot(HaveOccurred())

// Currently we wait for some time before removing the trunk from cache
// to allow evicted Pods's event to be received and their Branch ENIs be
// removed. In this period if we try to make the node managed again, it will
// fail
time.Sleep(branch.NodeDeleteRequeueRequestDelay)

By("adding the has trunk ENI label")
err = frameWork.NodeManager.AddLabels(targetedNodes,
map[string]string{config.HasTrunkAttachedLabel: "true"})
By("verifying node has CNINode present")
cniNode, err := frameWork.NodeManager.GetCNINode(&node)
Expect(err).ToNot(HaveOccurred())

By("creating the Pod on now managed node and verify it runs")
secondPod := podTemplate.DeepCopy()
secondPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, secondPod, utils.ResourceCreationTimeout)
Expect(err).ToNot(HaveOccurred())

verify.VerifyNetworkingOfPodUsingENI(*secondPod, []string{securityGroupID1})
Expect(cniNode.Name).To(Equal(node.Name))

// we don't support changing SGP managed node to unmanaged node
// after using CNINode, no longer like node label the feature in CNINode Spec shouldn't be modified
// only run this test for old label based mode
if !lo.ContainsBy(cniNode.Spec.Features, func(addedFeature cninode.Feature) bool {
return addedFeature.Name == cninode.SecurityGroupsForPods
}) {
if _, found := node.Labels[config.HasTrunkAttachedLabel]; found {
// This should never happens as once the trunk is attached,
// this label will not be removed again. This is for testing
// purposes to make a managed node an un-managed node
By("removing the has-trunk-attached label from the node")
err = frameWork.NodeManager.RemoveLabels(targetedNodes,
map[string]string{config.HasTrunkAttachedLabel: "true"})
Expect(err).To(HaveOccurred())

firstPod := podTemplate.DeepCopy()
By("creating a Pod on the un-managed node and verifying it fails")
_, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, firstPod, utils.ResourceCreationTimeout)
Expect(err).To(HaveOccurred())

By("deleting the pod")
err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, firstPod)
Expect(err).ToNot(HaveOccurred())

// Currently we wait for some time before removing the trunk from cache
// to allow evicted Pods's event to be received and their Branch ENIs be
// removed. In this period if we try to make the node managed again, it will
// fail
time.Sleep(branch.NodeDeleteRequeueRequestDelay)

By("adding the has trunk ENI label")
err = frameWork.NodeManager.AddLabels(targetedNodes,
map[string]string{config.HasTrunkAttachedLabel: "true"})
Expect(err).ToNot(HaveOccurred())

By("creating the Pod on now managed node and verify it runs")
secondPod := podTemplate.DeepCopy()
secondPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, secondPod, utils.ResourceCreationTimeout)
Expect(err).ToNot(HaveOccurred())

verify.VerifyNetworkingOfPodUsingENI(*secondPod, []string{securityGroupID1})

}
}
})
})

Expand All @@ -485,7 +521,7 @@ var _ = Describe("Branch ENI Pods", func() {
pod := podTemplate.DeepCopy()

By("creating pod which should not run since controller is down")
pod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, pod, time.Second*10)
_, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, pod, time.Second*10)
Expect(err).To(HaveOccurred())

By("scaling the controller deployment to 2")
Expand Down

0 comments on commit 97695bd

Please sign in to comment.