diff --git a/cmd/spiderpool/cmd/command_add.go b/cmd/spiderpool/cmd/command_add.go index 0695ee1c8a..079a747183 100644 --- a/cmd/spiderpool/cmd/command_add.go +++ b/cmd/spiderpool/cmd/command_add.go @@ -182,5 +182,9 @@ func assembleResult(cniVersion, IfName string, ipamResponse *daemonset.PostIpamI } } + if len(result.IPs) == 0 { + return nil, fmt.Errorf("no Interface %s IP allocation found", IfName) + } + return result, nil } diff --git a/pkg/ipam/pool_selections.go b/pkg/ipam/pool_selections.go index f65c845a76..fd90a86bce 100644 --- a/pkg/ipam/pool_selections.go +++ b/pkg/ipam/pool_selections.go @@ -49,7 +49,7 @@ func (i *ipam) getPoolCandidates(ctx context.Context, addArgs *models.IpamAddArg // Select IPPool candidates through the Pod annotation "ipam.spidernet.io/ippools". if anno, ok := pod.Annotations[constant.AnnoPodIPPools]; ok { - return getPoolFromPodAnnoPools(ctx, anno) + return getPoolFromPodAnnoPools(ctx, anno, *addArgs.IfName) } // Select IPPool candidates through the Pod annotation "ipam.spidernet.io/ippool". @@ -305,7 +305,7 @@ func (i *ipam) applyThirdControllerAutoPool(ctx context.Context, subnetName stri return pool, nil } -func getPoolFromPodAnnoPools(ctx context.Context, anno string) (ToBeAllocateds, error) { +func getPoolFromPodAnnoPools(ctx context.Context, anno, currentNIC string) (ToBeAllocateds, error) { logger := logutils.FromContext(ctx) logger.Sugar().Infof("Use IPPools from Pod annotation '%s'", constant.AnnoPodIPPools) @@ -317,7 +317,7 @@ func getPoolFromPodAnnoPools(ctx context.Context, anno string) (ToBeAllocateds, } // validate and mutate the IPPools annotation value - err = validateAndMutateMultipleNICAnnotations(annoPodIPPools) + err = validateAndMutateMultipleNICAnnotations(annoPodIPPools, currentNIC) if nil != err { return nil, fmt.Errorf("%w: %v", errPrefix, err) } diff --git a/pkg/ipam/utils.go b/pkg/ipam/utils.go index 663c6fe535..1049d632b4 100644 --- a/pkg/ipam/utils.go +++ b/pkg/ipam/utils.go @@ -188,16 +188,16 @@ func IsMultipleNicWithNoName(anno map[string]string) bool { return result } -func validateAndMutateMultipleNICAnnotations(annoIPPoolsValue types.AnnoPodIPPoolsValue) error { +func validateAndMutateMultipleNICAnnotations(annoIPPoolsValue types.AnnoPodIPPoolsValue, currentNIC string) error { if len(annoIPPoolsValue) == 0 { return fmt.Errorf("value requires at least one item") } nicSet := map[string]struct{}{} - isEmpty := annoIPPoolsValue[0].NIC == "" + hasNIC := annoIPPoolsValue[0].NIC != "" for index := range annoIPPoolsValue { // require all item NIC should be same in specified or unspecified. - if (annoIPPoolsValue[index].NIC == "") != isEmpty { + if (annoIPPoolsValue[index].NIC != "") != hasNIC { return fmt.Errorf("NIC should be either all specified or all unspecified") } @@ -209,10 +209,19 @@ func validateAndMutateMultipleNICAnnotations(annoIPPoolsValue types.AnnoPodIPPoo } // require no NIC name duplicated, this works for multiple NIC specified by the users - if _, ok := nicSet[annoIPPoolsValue[index].NIC]; ok { + if _, ok := nicSet[annoIPPoolsValue[index].NIC]; !ok { + nicSet[annoIPPoolsValue[index].NIC] = struct{}{} + } else { return fmt.Errorf("duplicate interface %s", annoIPPoolsValue[index].NIC) } } + if hasNIC { + _, ok := nicSet[currentNIC] + if !ok { + return fmt.Errorf("interface %s must be specified", currentNIC) + } + } + return nil } diff --git a/test/doc/annotation.md b/test/doc/annotation.md index 6618a854ea..e603328c31 100644 --- a/test/doc/annotation.md +++ b/test/doc/annotation.md @@ -1,7 +1,7 @@ # E2E Cases for Annotation | Case ID | Title | Priority | Smoke | Status | Other | -| ------- |-----------------------------------------------------------------------------------------------------------------------------|----------|-------|--------|-------| +|---------|-----------------------------------------------------------------------------------------------------------------------------|----------|-------|--------|-------| | A00001 | It fails to run a pod with different VLANs for IPv4 and IPv6 IPPools | p3 | | done | | | A00002 | Added fields such as `"dist":"1.0.0.0/16"`, `"gw":"1.0.0.1"`, and `nics` and the pod was running successfully | p2 | | done | | | A00003 | Failed to run a pod with invalid annotations | p3 | | done | | @@ -13,3 +13,5 @@ | A00010 | Modify the annotated IPPool for a pod running on multiple NICs | p3 | | done | | | A00011 | Use the ippool route with `cleanGateway=false` in the pod annotation as a default route | p3 | | done | | | A00012 | Specify the default NIC through Pod annotations | p2 | | | | +| A00013 | It's invalid to specify one NIC corresponding IPPool in IPPools annotation with multiple NICs | p2 | | done | | +| A00014 | It's invalid to specify same NIC name for IPPools annotation with multiple NICs | p2 | | done | | diff --git a/test/e2e/annotation/annotation_test.go b/test/e2e/annotation/annotation_test.go index ada4359d34..4bdbb4962c 100644 --- a/test/e2e/annotation/annotation_test.go +++ b/test/e2e/annotation/annotation_test.go @@ -740,6 +740,71 @@ var _ = Describe("test annotation", Label("annotation"), func() { }) }) + + Context("wrong IPPools annotation usage", func() { + It("It's invalid to specify one NIC corresponding IPPool in IPPools annotation with multiple NICs", Label("A00013"), func() { + // set pod annotation for nics + podIppoolsAnno := types.AnnoPodIPPoolsValue{ + { + NIC: common.NIC2, + }, + } + if frame.Info.IpV4Enabled { + podIppoolsAnno[0].IPv4Pools = []string{common.SpiderPoolIPv4SubnetVlan100} + } + if frame.Info.IpV6Enabled { + podIppoolsAnno[0].IPv6Pools = []string{common.SpiderPoolIPv6SubnetVlan100} + } + podIppoolsAnnoMarshal, err := json.Marshal(podIppoolsAnno) + Expect(err).NotTo(HaveOccurred()) + podYaml := common.GenerateExamplePodYaml(podName, nsName) + podYaml.Annotations = map[string]string{ + pkgconstant.AnnoPodIPPools: string(podIppoolsAnnoMarshal), + common.MultusNetworks: fmt.Sprintf("%s/%s", common.MultusNs, common.MacvlanVlan100), + } + GinkgoWriter.Printf("succeeded to generate pod yaml with IPPools annotation: %+v. \n", podYaml) + + Expect(frame.CreatePod(podYaml)).To(Succeed()) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute*1) + defer cancel() + GinkgoWriter.Printf("wait for one minute that pod %v/%v would not ready. \n", nsName, podName) + _, err = frame.WaitPodStarted(podName, nsName, ctx) + Expect(err).To(HaveOccurred()) + }) + + It("It's invalid to specify same NIC name for IPPools annotation with multiple NICs", Label("A00014"), func() { + // set pod annotation for nics + podIppoolsAnno := types.AnnoPodIPPoolsValue{ + { + NIC: common.NIC2, + }, + { + NIC: common.NIC2, + }, + } + if frame.Info.IpV4Enabled { + podIppoolsAnno[0].IPv4Pools = []string{common.SpiderPoolIPv4SubnetVlan100} + } + if frame.Info.IpV6Enabled { + podIppoolsAnno[0].IPv6Pools = []string{common.SpiderPoolIPv6SubnetVlan100} + } + podIppoolsAnnoMarshal, err := json.Marshal(podIppoolsAnno) + Expect(err).NotTo(HaveOccurred()) + podYaml := common.GenerateExamplePodYaml(podName, nsName) + podYaml.Annotations = map[string]string{ + pkgconstant.AnnoPodIPPools: string(podIppoolsAnnoMarshal), + common.MultusNetworks: fmt.Sprintf("%s/%s", common.MultusNs, common.MacvlanVlan100), + } + GinkgoWriter.Printf("succeeded to generate pod yaml with same NIC name annotation: %+v. \n", podYaml) + + Expect(frame.CreatePod(podYaml)).To(Succeed()) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute*1) + defer cancel() + GinkgoWriter.Printf("wait for one minute that pod %v/%v would not ready. \n", nsName, podName) + _, err = frame.WaitPodStarted(podName, nsName, ctx) + Expect(err).To(HaveOccurred()) + }) + }) }) func checkAnnotationPriority(podYaml *corev1.Pod, podName, nsName string, v4PoolNameList, v6PoolNameList []string) {