-
Notifications
You must be signed in to change notification settings - Fork 4.8k
CORENET-6363: Add PreconfiguredUDNAddresses duplicate IP detection tests #30197
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ import ( | |
| "strings" | ||
| "time" | ||
|
|
||
| "sigs.k8s.io/yaml" | ||
|
|
||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| e2ekubectl "k8s.io/kubernetes/test/e2e/framework/kubectl" | ||
|
|
||
|
|
@@ -93,6 +95,81 @@ func (c *Client) GetJSONPath(resource, name, jsonPath string) (string, error) { | |
| } | ||
| return strings.TrimSuffix(strings.TrimPrefix(output, `"`), `"`), nil | ||
| } | ||
|
|
||
| func (c *Client) GetPodsByLabel(labelKey, labelValue string) ([]string, error) { | ||
| output, err := c.oc.AsAdmin().Run("get").Args("pods", "-n", c.oc.Namespace(), "-l", fmt.Sprintf("%s=%s", labelKey, labelValue), "-o", "name").Output() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if output == "" { | ||
| return []string{}, nil | ||
| } | ||
|
|
||
| lines := strings.Split(strings.TrimSpace(output), "\n") | ||
| podNames := make([]string, 0, len(lines)) | ||
| for _, line := range lines { | ||
| if line != "" { | ||
| podName := strings.TrimPrefix(line, "pod/") | ||
| podNames = append(podNames, podName) | ||
| } | ||
| } | ||
| return podNames, nil | ||
| } | ||
|
|
||
| func (c *Client) GetEventsForPod(podName string) ([]string, error) { | ||
| output, err := c.oc.AsAdmin().Run("get").Args("events", "-n", c.oc.Namespace(), "--field-selector", fmt.Sprintf("involvedObject.name=%s,involvedObject.kind=Pod", podName), "-o", "custom-columns=MESSAGE:.message", "--no-headers").Output() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: same as above, this should work:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do in follow up PR |
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if output == "" { | ||
| return []string{}, nil | ||
| } | ||
| lines := strings.Split(strings.TrimSpace(output), "\n") | ||
| messages := make([]string, 0, len(lines)) | ||
| for _, line := range lines { | ||
| if line != "" { | ||
| messages = append(messages, line) | ||
| } | ||
| } | ||
| return messages, nil | ||
| } | ||
|
|
||
| type Option func(map[string]interface{}) | ||
|
|
||
| func (c *Client) CreateVMIFromSpec(vmNamespace, vmName string, vmiSpec map[string]interface{}, opts ...Option) error { | ||
| newVMI := map[string]interface{}{ | ||
| "apiVersion": "kubevirt.io/v1", | ||
| "kind": "VirtualMachineInstance", | ||
| "metadata": map[string]interface{}{ | ||
| "name": vmName, | ||
| "namespace": vmNamespace, | ||
| }, | ||
| "spec": vmiSpec, | ||
| } | ||
|
|
||
| for _, opt := range opts { | ||
| opt(newVMI) | ||
| } | ||
|
|
||
| newVMIYAML, err := yaml.Marshal(newVMI) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
Comment on lines
+140
to
+157
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't we use Unstructured stuff for this ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure I follow - can you elaborate? |
||
|
|
||
| return c.Apply(string(newVMIYAML)) | ||
| } | ||
|
|
||
| func WithAnnotations(annotations map[string]string) Option { | ||
| return func(cr map[string]interface{}) { | ||
| metadata, hasMetadata := cr["metadata"].(map[string]interface{}) | ||
| if !hasMetadata { | ||
| metadata = make(map[string]interface{}) | ||
| cr["metadata"] = metadata | ||
| } | ||
| metadata["annotations"] = annotations | ||
| } | ||
| } | ||
|
|
||
| func ensureVirtctl(oc *exutil.CLI, dir string) (string, error) { | ||
| filepath := filepath.Join(dir, "virtctl") | ||
| _, err := os.Stat(filepath) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,8 @@ import ( | |
| "github.com/openshift/origin/test/extended/util/image" | ||
| ) | ||
|
|
||
| const kvIPRequestsAnnot = "network.kubevirt.io/addresses" | ||
|
|
||
| var _ = Describe("[sig-network][OCPFeatureGate:PersistentIPsForVirtualization][Feature:Layer2LiveMigration] Kubevirt Virtual Machines", func() { | ||
| // disable automatic namespace creation, we need to add the required UDN label | ||
| oc := exutil.NewCLIWithoutNamespace("network-segmentation-e2e") | ||
|
|
@@ -303,6 +305,20 @@ var _ = Describe("[sig-network][OCPFeatureGate:PersistentIPsForVirtualization][F | |
| preconfiguredMAC: "02:0A:0B:0C:0D:51", | ||
| }, | ||
| ), | ||
| Entry( | ||
| "[OCPFeatureGate:PreconfiguredUDNAddresses] when the VM with preconfigured IP address is created when the address is already taken", | ||
| networkAttachmentConfigParams{ | ||
| name: nadName, | ||
| topology: "layer2", | ||
| role: "primary", | ||
| allowPersistentIPs: true, | ||
| }, | ||
| kubevirt.FedoraVMWithPreconfiguredPrimaryUDNAttachment, | ||
| duplicateVM, | ||
| workloadNetworkConfig{ | ||
| preconfiguredIPs: []string{"203.203.0.100", "2014:100:200::100"}, | ||
| }, | ||
| ), | ||
| ) | ||
| }, | ||
| Entry("NetworkAttachmentDefinitions", func(c networkAttachmentConfigParams) { | ||
|
|
@@ -526,6 +542,65 @@ func verifyVMMAC(virtClient *kubevirt.Client, vmName, expectedMAC string) { | |
| Should(Equal(expectedMAC)) | ||
| } | ||
|
|
||
| func duplicateVM(cli *kubevirt.Client, vmNamespace, vmName string) { | ||
| GinkgoHelper() | ||
| duplicateVMName := vmName + "-duplicate" | ||
| By(fmt.Sprintf("Duplicating VM %s/%s to %s/%s", vmNamespace, vmName, vmNamespace, duplicateVMName)) | ||
|
|
||
| vmiSpecJSON, err := cli.GetJSONPath("vmi", vmName, "{.spec}") | ||
| Expect(err).NotTo(HaveOccurred()) | ||
| var vmiSpec map[string]interface{} | ||
| Expect(json.Unmarshal([]byte(vmiSpecJSON), &vmiSpec)).To(Succeed()) | ||
|
|
||
| originalVMIRawAnnotations, err := cli.GetJSONPath("vmi", vmName, "{.metadata.annotations}") | ||
| Expect(err).NotTo(HaveOccurred()) | ||
|
|
||
| originalVMIAnnotations := map[string]string{} | ||
| Expect(json.Unmarshal([]byte(originalVMIRawAnnotations), &originalVMIAnnotations)).To(Succeed()) | ||
|
|
||
| var vmiCreationOptions []kubevirt.Option | ||
| var vmiExpectations []func() | ||
| if requestedIPs, hasIPRequests := originalVMIAnnotations[kvIPRequestsAnnot]; hasIPRequests { | ||
| vmiCreationOptions = append( | ||
| vmiCreationOptions, | ||
| kubevirt.WithAnnotations(ipRequests(requestedIPs)), | ||
| ) | ||
| vmiExpectations = append(vmiExpectations, func() { | ||
| waitForVMPodEventWithMessage( | ||
| cli, | ||
| vmNamespace, | ||
| duplicateVMName, | ||
| "IP is already allocated", | ||
| 2*time.Minute, | ||
| ) | ||
| }) | ||
| } | ||
| Expect(cli.CreateVMIFromSpec(vmNamespace, duplicateVMName, vmiSpec, vmiCreationOptions...)).To(Succeed()) | ||
| for _, expectation := range vmiExpectations { | ||
| expectation() | ||
| } | ||
| } | ||
|
|
||
| func waitForVMPodEventWithMessage(vmClient *kubevirt.Client, vmNamespace, vmName, expectedEventMessage string, timeout time.Duration) { | ||
| GinkgoHelper() | ||
| By(fmt.Sprintf("Waiting for event containing %q on VM %s/%s virt-launcher pod", expectedEventMessage, vmNamespace, vmName)) | ||
|
|
||
| Eventually(func(g Gomega) []string { | ||
| const vmLabelKey = "vm.kubevirt.io/name" | ||
| podNames, err := vmClient.GetPodsByLabel(vmLabelKey, vmName) | ||
| g.Expect(err).NotTo(HaveOccurred(), "Failed to get pods by label %s=%s", vmLabelKey, vmName) | ||
| g.Expect(podNames).To(HaveLen(1), "Expected exactly one virt-launcher pod for VM %s/%s, but found %d pods: %v", vmNamespace, vmName, len(podNames), podNames) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: don't we need to filter out some states ? Let's take the "migration" scenario into consideration; a completed migration will leave the original (src) pod as Do we want to address that scenario here ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. depends.. Right now we not looking for event of vm-pods during migration/restart, so I would keep things simple.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fair enough for me. |
||
|
|
||
| virtLauncherPodName := podNames[0] | ||
| eventMessages, err := vmClient.GetEventsForPod(virtLauncherPodName) | ||
| g.Expect(err).NotTo(HaveOccurred(), "Failed to get events for pod %s", virtLauncherPodName) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure we want to retry o error ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not? if k8s service is down or something we should retry.. My reasoning is: it's not what's the test is about, we should try and get the events and fail only if the relevant even isn't there. |
||
|
|
||
| return eventMessages | ||
| }).WithPolling(time.Second).WithTimeout(timeout).Should( | ||
| ContainElement(ContainSubstring(expectedEventMessage)), | ||
| fmt.Sprintf("Expected to find an event containing %q", expectedEventMessage)) | ||
| } | ||
|
|
||
| func waitForPodsCondition(fr *framework.Framework, pods []*corev1.Pod, conditionFn func(g Gomega, pod *corev1.Pod)) { | ||
| for _, pod := range pods { | ||
| Eventually(func(g Gomega) { | ||
|
|
@@ -696,3 +771,7 @@ func formatAddressesAnnotation(preconfiguredIPs []string) (string, error) { | |
|
|
||
| return string(staticIPs), nil | ||
| } | ||
|
|
||
| func ipRequests(ips string) map[string]string { | ||
| return map[string]string{kvIPRequestsAnnot: ips} | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: Why do need an admin?
Why can't you use a lister?
it would mean that there is no need to parse the output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was following the logic of former code. So in order to fix this properly, I will issue a follow-up PR to fix it in all client functions