e2e, Introduce e2e test: resolving a FQDN entry #38

AlonaKaplan · 2022-12-04T10:52:16Z

The test is creating a VMI with one multus and no default interface and verifies that nslookup to the interface fqdn retrieves the correct IP.

The PR also contains -

Enable deploying kubevirt and cnao on cluster-up.
Changing the number of nodes in the ci cluster to 1 (it is enough for now).
Updating the functest.sh to run the e2e test.

kubevirt-bot · 2022-12-04T10:52:18Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: Alona Paz <alkaplan@redhat.com>

Currently one node is enough for us. The cluster has forwarding from 127.0.0.1:31111 to the node01. By having only one node we make sure the dns query is forwarding to the correct node. Signed-off-by: Alona Paz <alkaplan@redhat.com>

Those are needed for the e2e test wihich creates a VM with multus interface. Signed-off-by: Alona Paz <alkaplan@redhat.com>

oshoval · 2022-12-05T07:57:53Z

About nslookup binary
I think we should do the following as part of hack/functest.sh

[ "$CI" == "true" ] && dnf install bind-utils (*)

if ! nslookup www.google.com > /dev/null 2>&1; then
  echo error
  exit 1
fi

So on both local and on CI, it will directly fail as first step if the binary doesn't exists
the (*) will make it work on CI for now of course, and once we add it to the bootstrap image we can remove it.
It might take time until it is merged, and a new image + bump exists, so IMO it is more comfortable to use (*) first
and do it in the bootstrap in parallel.
We should not install stuff locally as part of the e2e tests, nor require sudo to run the tests.
On CI we don't need sudo.
bootstrap file images/bootstrap/Dockerfile

oshoval

Nice thanks
First pass

Please update PR desc to
e2e, Introduce e2e test: resolving a FQDN entry
or something like that

typo on the last line of the PR desc (file name missing dot)

cluster/up.sh

oshoval · 2022-12-05T08:04:22Z

hack/functest.sh

@@ -1,3 +1,8 @@
-#!/bin/bash -e
+#!/usr/bin/env bash


please keep the previous shebang, this is the one we use all over

plus remove plesase the set -xe

oshoval · 2022-12-05T08:06:13Z

tests/vm_startup_test.go

+const pollingInterval = 5 * time.Second
+const testNamespace = "secondary-test"
+const dnsPort = "31111"
+const dnsIP = "127.0.0.1" // Forwarded to the node port - https://github.com/kubevirt/kubevirtci/pull/867 ,https://github.com/kubevirt/kubesecondarydns/pull/23


I dont think we need the comments

There is no chance that external contributor or even future us will understand why "127.0.0.1" is the dnsIP without thos comment.

so maybe just something generic as this is the flow of kubevirtci ?
or even just the 2nd PR can be enough ?
two PRs is way too much imo

I think those two PR explain the best how we rich the dns. I prefer to have it and to same future developer some time in the future.

I am personally against too much comments / links
if we can find something in between it would be great please
btw the first link has info about the node port as well

note that even now the PRs are outdated, because the was another PR which change kuevirtci default host port from 53 to 31111
this is the problem with comments

I think we can mention here the README.md, that once the PR of readme is merged, it explains a bit about the topology we are using on kubevirtci / nodeport

oshoval · 2022-12-05T08:07:33Z

tests/vm_startup_test.go

+			Config: `{
+      "cniVersion": "0.3.1",
+      "name": "mynet",
+      "plugins": [
+        {
+          "type": "ptp",
+          "ipam": {
+		      "type": "static",
+		      "addresses": [
+			      {
+				       "address": "10.10.0.5/24",
+				       "gateway": "10.10.0.254"
+			      }
+		      ]
+	      }
+        }
+      ]
+}`,


maybe lets do it go style?
unless it required lots of packages

what do you mean by go style?

like the VMI
https://github.com/kubevirt/kubesecondarydns/pull/38/files#diff-738c6c49cffa4043869737c9b30a528211b64516acbf533de44098f593e8b4d1R17

a go struct that is filled with values

the config is a sting.

Right maybe better to declare it as a string on top and use it ? (like kubevirt's newNetworkAttachmentDefinition)

btw alternative indentations if you prefer
This one is what user-guide of kubevirt use

config: '{ "cniVersion": "0.3.1", "name": "mynet", "plugins": [ { "type": "ptp", "ipam": { "type": "static", "addresses": [ { "address": "10.10.0.5/24", "gateway": "10.10.0.254" } ] } } ] }'

This one is what yq pretty prints
(on kubevirt we also have one liners like this)

'{ "cniVersion": "0.3.1", "name": "mynet", "plugins": [ { "type": "ptp", "ipam": { "type": "static", "addresses": [ { "address": "10.10.0.5/24", "gateway": "10.10.0.254" } ] } } ] }'

I find the one liner less readable.

what about the fixed ident ?

oshoval · 2022-12-05T08:11:06Z

tests/vm_startup_test.go

+const testNamespace = "secondary-test"
+const dnsPort = "31111"
+const dnsIP = "127.0.0.1" // Forwarded to the node port - https://github.com/kubevirt/kubevirtci/pull/867 ,https://github.com/kubevirt/kubesecondarydns/pull/23
+const domain = "vm.secondary.io"


Please consider #36
and then it should be just "vm"

Well, it's not merged yet so I cannot consider it:) #36 will have to fix the e2e if the e2e will be merged first.

can you review it please and lets merge it ? it is an easy one and lgtmed

oshoval · 2022-12-05T08:17:35Z

hack/functest.sh

+source ./cluster/cluster.sh
+
+export KUBEVIRT_CLIENT_GO_SCHEME_REGISTRATION_VERSION=v1
+KUBECONFIG=${KUBECONFIG:-$(cluster::kubeconfig)} $GO test -test.timeout=1h -race -test.v ./tests/... $E2E_TEST_ARGS -ginkgo.v


we have twice -v, lets remove the deprecated one

which one of the is deprecated?

I think the ginkgo one
best to try please the different ways

oshoval · 2022-12-05T08:19:47Z

tests/vm_startup_test.go

+	BeforeEach(func() {
+		By("Creating test namespace")
+		ns := k8sv1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: testNamespace,
+			},
+		}
+		err := getClient().Create(context.Background(), &ns)
+		Expect(err).ToNot(HaveOccurred(), "couldn't create namespace for the test")
+	})


Can be done as BeforeAll / BeforeSuite with a defer to cleanup it instead of the AfterEach
On the other hand, it guarantee the leftovers are cleaned if any, but implicit

we usually don't delete the NS after each test

Since we have only one test, and in the future will maybe have few test. I find this approach cleaner and safer.

It is an overkill imo, anyhow we can refactor it later, but imo we should

oshoval · 2022-12-05T08:20:23Z

tests/vm_startup_test.go

+			Expect(err).ToNot(HaveOccurred(), "Should successfully create the NAD")
+
+			By("Creating a VirtualMachineInstance")
+			interfaceName := "nic1"


please use a const for it

oshoval · 2022-12-05T08:20:35Z

tests/vm_startup_test.go

+			interfaces := []v1.Interface{{Name: interfaceName, InterfaceBindingMethod: v1.InterfaceBindingMethod{Bridge: &v1.InterfaceBridge{}}}}
+			networks := []v1.Network{
+				{Name: interfaceName, NetworkSource: v1.NetworkSource{
+					Multus: &v1.MultusNetwork{NetworkName: "ptp-conf"},


please use a const for it

why? It is never reused.

it is nicer to have a const even for one time variable which declares a global name

oshoval · 2022-12-05T08:20:41Z

tests/vm_startup_test.go

+					Multus: &v1.MultusNetwork{NetworkName: "ptp-conf"},
+				}},
+			}
+			vmi := CreateVmiObject("vmi-sec", testNamespace, interfaces, networks)


please use a const for it

why? It is never reused.

it is nicer to have a const even for one time variable which declares a global name

oshoval · 2022-12-05T08:28:22Z

We need to remove push or pull_request from the git actions
when you create a PR it runs both, maybe because you are the original repo creator or something
(it doesn't happen when PRs are created by other people)

oshoval · 2022-12-05T08:32:57Z

tests/vm_generator.go

+		Domain: v1.DomainSpec{
+			Resources: v1.ResourceRequirements{
+				Requests: k8sv1.ResourceList{
+					k8sv1.ResourceMemory: resource.MustParse("64M"),


Please use 128Mi
64M is not enough
see please kubevirt/kubevirt#5039
especially cirros-dev/cirros#52 (comment)

oshoval · 2022-12-05T08:35:09Z

hack/functest.sh

+source ./cluster/cluster.sh
+
+export KUBEVIRT_CLIENT_GO_SCHEME_REGISTRATION_VERSION=v1
+KUBECONFIG=${KUBECONFIG:-$(cluster::kubeconfig)} $GO test -test.timeout=1h -race -test.v ./tests/... $E2E_TEST_ARGS -ginkgo.v


please add deterministic seed of 0 (unless we do want tests to run on random order once we have more)
on kubevirt we use fixed seed

Currently we have just one test, so it is not relevant.
Anyway, I think it is healthier to run the tests in a random order.

AlonaKaplan · 2022-12-05T08:36:04Z

[ "$CI" == "true" ] && dnf install bind-utils (*)
if ! nslookup www.google.com > /dev/null 2>&1; then
  echo error
  exit 1
fi

Thanks, updated the PR

oshoval · 2022-12-05T08:46:04Z

/release-note-none

oshoval · 2022-12-05T09:18:28Z

Need to show this to Brian please
https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/pr-logs/pull/kubevirt_kubesecondarydns/38/pull-kubesecondarydns-e2e-k8s/1599692018617946112

I think there is a new image with extended timeout
will try to update job to v20221119-67aebd9

kubevirt/project-infra#2500

oshoval · 2022-12-05T09:23:01Z

/test pull-kubesecondarydns-e2e-k8s

i hope btw sudo is not needed, lets see

oshoval · 2022-12-05T09:36:18Z

hack/functest.sh

-echo hello world
+source ./cluster/cluster.sh
+
+[ "$CI" == "true" ] && dnf install bind-utils


oshoval

do we still need pkg/controllers/controllers_suite_test.go
or it can be deleted ?

tests/client.go

oshoval · 2022-12-05T11:58:35Z

Please fork the repo for next PRs, it seems you are using the main repo branches
https://github.com/kubevirt/kubesecondarydns/branches
This is why it runs both pull_request and push git actions.

oshoval · 2022-12-05T12:09:11Z

hack/functest.sh

+  exit 1
+fi
+
+KUBECONFIG=${KUBECONFIG:-$(cluster::kubeconfig)} $GO test -test.timeout=1h -test.v ./tests/... -ginkgo.v


i think we need to add no-color please
else we see the control characters on the log

https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/pr-logs/pull/kubevirt_kubesecondarydns/38/pull-kubesecondarydns-e2e-k8s/1599451475841388544
Can you please point me what control characters do we see here?

watch the raw one
https://storage.googleapis.com/kubevirt-prow/pr-logs/pull/kubevirt_kubesecondarydns/38/pull-kubesecondarydns-e2e-k8s/1599451475841388544/build-log.txt

well it will remove colors from the spyglass one (the one in the link you pasted)
IIRC this what we are doing on other repos, --no-color (but best to double check)

�[38;5;243mBegin Captured GinkgoWriter Output >>�[0m �[1mSTEP:�[0m Creating test namespace �[38;5;243m12/04/22 17:18:33.833�[0m �[1mSTEP:�[0m Creating a NetworkAttachmentDefinition �[38;5;243m12/04/22 17:18:34.402�[0m �[1mSTEP:�[0m Creating a VirtualMachineInstance �[38;5;243m12/04/22 17:18:34.409�[0m �[1mSTEP:�[0m Invoking nslookup on the VMI interface FQDN �[38;5;243m12/04/22 17:18:34.437�[0m �[1mSTEP:�[0m Removing all Virtual Machine Instances from the namespace �[38;5;243m12/04/22 17:19:44.468�[0m �[1mSTEP:�[0m Removing all Network AttachmentDefinitions from the namespace �[38;5;243m12/04/22 17:19:54.501�[0m �[1mSTEP:�[0m Removing the test namespace �[38;5;243m12/04/22 17:19:54.509�[0m �[38;5;243m<< End Captured GinkgoWriter Output�[0m

We don't have the no-color flag in kubevirt. And I find it nice to see the colors. I prefer to keep it.

automation/test.sh:ginko_params="--no-color --seed=42"
automation/repeated_test.sh: ginko_params="-no-color -succinct -slow-spec-threshold=30s -focus=${NEW_TESTS} -skip=QUARANTINE -regexScansFilePath=true"

from kubevirt

Moreover spyglass itself mark the important words like FAIL/FATAL/ERROR and so on IIRC

oshoval · 2022-12-05T12:21:39Z

tests/vm_startup_test.go

+const testNamespace = "secondary-test"
+const dnsPort = "31111"
+const dnsIP = "127.0.0.1" // Forwarded to the node port - https://github.com/kubevirt/kubevirtci/pull/867 ,https://github.com/kubevirt/kubesecondarydns/pull/23
+const domain = "vm.secondary.io"


what about reading it from the CR and building accordingly the expected string?
this way it will work for:

empty string

D/S that fill the cluster-name (if this what we are going to do at the end)

non empty string

I think it is an overkill for the test. If we will ever need it on d/s, then we can change it.

I think that this test might even help us to test it D/S manually
but ok we can just update upon need,
what about reading it from os.getenv that allows to config it easily when running D/S ?

EDIT - better to wait indeed, because it is not the only thing that will need to be changed
i.e IP:port

Let's think about it when we will get to d/s

AlonaKaplan · 2022-12-05T12:47:20Z

do we still need pkg/controllers/controllers_suite_test.go or it can be deleted ?

We need it. It is for the controller unit tests.

oshoval · 2022-12-05T14:59:50Z

Please note this test would run also as part of CNAO
CNAO deploys KSD and calls make create-nodeport, and then vendor the repo and run the make functest target.
so if there is something that related to kubeconfig / namespace etc
it should be dynamic (about kubeconfig saw you already took care)

AlonaKaplan · 2022-12-06T06:52:23Z

I use test namespace. It should be ok.

oshoval · 2022-12-06T07:01:19Z

Once kubevirt/project-infra#2504 is merged we can remove the dnf install line please

oshoval

Thanks, few more small comments

about PR desc, nodes reducing - maybe worth to either explain why, or at least tell see (specific) commit for more info

oshoval · 2022-12-06T07:17:04Z

tests/vm_generator.go

+		Name: "cloudinitdisk",
+		DiskDevice: v1.DiskDevice{
+			Disk: &v1.DiskTarget{
+				Bus: "virtio",


we use virtio in couple places in the same file. worth to add const please

oshoval · 2022-12-06T07:21:26Z

tests/vm_startup_test.go

+		vmiList := &v1.VirtualMachineInstanceList{}
+		err := getClient().List(context.Background(), vmiList, &client.ListOptions{Namespace: testNamespace})
+		Expect(err).ToNot(HaveOccurred())
+
+		for _, vmiObject := range vmiList.Items {
+			err = getClient().Delete(context.Background(), &vmiObject)
+			Expect(err).ToNot(HaveOccurred())
+		}


isn't there a way to just delete in one shot all vms from a given namespace using the client ?
(if so, same for nads etc)

Note that the Expect in the for means that if an error occur, it will skip the rest,
i guess it is fine for now, maybe in the future we would prefer to collect errors but continue and assert the errors at the end

Since I'm removing the namespace, remvoing the nads and vmis is redudnant anyway. Removed it.

Its a bit not what we are doing usually, but it does the job
fine for now, later we can update if needed
(note that it does stuff implicit)

oshoval · 2022-12-06T07:22:17Z

tests/vm_startup_test.go

+		err := getClient().List(context.Background(), nadList, &client.ListOptions{Namespace: testNamespace})
+		Expect(err).ToNot(HaveOccurred())
+
+		for _, nadObject := range nadList.Items {


we have just one nad, can we simplify this AfterEach please ?

oshoval · 2022-12-06T07:23:02Z

tests/vm_startup_test.go

+			err := getClient().List(context.Background(), vmiList, &client.ListOptions{Namespace: testNamespace})
+			Expect(err).ToNot(HaveOccurred())
+			return len(vmiList.Items)
+


please drop this blank line
(same for line 80)

oshoval · 2022-12-06T07:23:54Z

tests/vm_startup_test.go

+			vmiList := &v1.VirtualMachineInstanceList{}
+			err := getClient().List(context.Background(), vmiList, &client.ListOptions{Namespace: testNamespace})
+			Expect(err).ToNot(HaveOccurred())
+			return len(vmiList.Items)


maybe return vmiList.Items and use BeEmpty ?
(same for the rest)

it was removed

oshoval · 2022-12-06T07:25:16Z

tests/vm_startup_test.go

+		Eventually(func() error {
+			return getClient().Delete(context.Background(), namespace)
+		}, 240*time.Second, 1*time.Second).Should(SatisfyAll(HaveOccurred(), WithTransform(errors.IsNotFound, BeTrue())), fmt.Sprintf("should successfully delete namespace '%s'", testNamespace))


Better and nicer please to delete once, and wait that get shows it is gone, as done with the rest
not delete in a loop as done now

The code is more compact this way. What are the benefits of the proposed approach?

It isn't correct to Delete Delete .. until gone
it is correct to Delete, Get Get until gone imo

oshoval · 2022-12-06T07:26:24Z

tests/vm_startup_test.go

+			By("Creating a NetworkAttachmentDefinition")
+			err := createNetworkAttachmentDefinition(testNamespace, "ptp-conf")
+			Expect(err).ToNot(HaveOccurred(), "Should successfully create the NAD")


can be done please as a BeforeEach of this It (in the same Context), as it is not part of the "It" but preparation,
makes the It smaller

oshoval · 2022-12-06T07:44:23Z

tests/vm_generator.go

+func CreateVmiObject(name string, namespace string, interfaces []v1.Interface, networks []v1.Network) *v1.VirtualMachineInstance {
+	vmi := getBaseVMI(randName(name))
+	vmi.Namespace = namespace
+	addContainerDisk(&vmi.Spec, "kubevirt/cirros-container-disk-demo:latest")


do we need maybe to add quay.io ? so it won't take from docker.io which we might stop updating ?

Note that the quay.io one is very old (2 years), the new ones have tag and are pretty newer (the "latest" might be an old os version that we don't use anymore)
I am not sure about what the docker.io has
we might want to consider later to upload a specific image or select a specific tag / specific image repo (such as quay.io)
pinning is also better than using latest.
anyhow we can postpone it if desired

I would add it to our local registry.

Still you need to select which one - this is the important part.
and imo no need to add it to the local, as it you are fetching it anyhow
but whatever you prefer

changed to quay.io/kubevirt/cirros-container-disk-demo:develalthough it doesn't really matter what image we are using. The devel one is maintained by kubevirt is should be enough for us.

oshoval · 2022-12-06T08:39:28Z

hack/functest.sh

-echo hello world
+source ./cluster/cluster.sh
+
+[ "$CI" == "true" ] && dnf install -y bind-utils


Please remove this line as kubevirt/project-infra#2504 is merged and includes
bind-utils

AlonaKaplan · 2022-12-06T10:18:56Z

Thanks, few more small comments

about PR desc, nodes reducing - maybe worth to either explain why, or at least tell see (specific) commit for more info

The relevant commit message has an explanation. Do you think it is not enough?

oshoval · 2022-12-06T10:22:17Z

Thanks, few more small comments
about PR desc, nodes reducing - maybe worth to either explain why, or at least tell see (specific) commit for more info

The relevant commit message has an explanation. Do you think it is not enough?

It is enough, but the line of the PR desc better to suggest to look on it please
Changing the number of nodes in the ci cluster to 1 (it is enough for now).
to
Changing the number of nodes of the cluster to 1 (see 2nd commit).

You might want to amend the 2nd commit by adding that the reason the NodePort doesn't work on the first node, is that in case there are more than 1 nodes, that node is not a worker and only workers are included as you found.
Alternatively also to add this info to the PR desc itself.

oshoval · 2022-12-06T12:22:16Z

tests/vm_startup_test.go

+
+const timeout = 3 * time.Minute
+const pollingInterval = 5 * time.Second
+const testNamespacePrefix = "secondary-test"


why we need this with the random suffix ?
even if we run on parallel, there will be a namespace per instance as kubevirt is doing

Currently we have just one test so we can use whatever namespace.
In the future, if we have more than one test we can continue to the next test without waiting for the previous test resources to be removed.

It is not a good practice imo, and i less like the way using different namespaces
I prefer that we delete VMI and not wait for them but not delete the NS as we are doing on kubevirt
note that on kubevirt there are some places where we do wait for all resources to gone

oshoval · 2022-12-06T12:26:11Z

tests/vm_generator.go

+	v1 "kubevirt.io/api/core/v1"
+)
+
+const virtio = "virtio"


nit: const VirtIO
(this is like we are doing it on kubevirt)

oshoval · 2022-12-06T12:28:34Z

tests/vm_startup_test.go

+var testNamespace string
+
+var _ = Describe("Virtual Machines Startup", func() {
+


nit: please drop the blank line

Signed-off-by: Alona Paz <alkaplan@redhat.com>

The script also makes sure `nslookup` is installed. Signed-off-by: Alona Paz <alkaplan@redhat.com>

oshoval

Thanks

AlonaKaplan · 2022-12-06T13:55:47Z

/approve

kubevirt-bot · 2022-12-06T13:55:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlonaKaplan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [AlonaKaplan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

AlonaKaplan added 3 commits December 4, 2022 15:39

rename suit_test to controllers_suite_test

38207e1

Signed-off-by: Alona Paz <alkaplan@redhat.com>

Change the number of cluster nodes to 1

aaf75c4

Currently one node is enough for us. The cluster has forwarding from 127.0.0.1:31111 to the node01. By having only one node we make sure the dns query is forwarding to the correct node. Signed-off-by: Alona Paz <alkaplan@redhat.com>

Deploy kubevirt and cnao of make cluster-sync

04cdd9a

Those are needed for the e2e test wihich creates a VM with multus interface. Signed-off-by: Alona Paz <alkaplan@redhat.com>

AlonaKaplan force-pushed the firt_e2e branch from 659f360 to 2ea6328 Compare December 4, 2022 17:09

AlonaKaplan marked this pull request as ready for review December 4, 2022 17:12

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2022

kubevirt-bot requested review from dteplits and oshoval December 4, 2022 17:12

oshoval reviewed Dec 5, 2022

View reviewed changes

AlonaKaplan force-pushed the firt_e2e branch from 2ea6328 to 3a9075d Compare December 5, 2022 08:35

AlonaKaplan force-pushed the firt_e2e branch from 3a9075d to 36aed3b Compare December 5, 2022 08:39

AlonaKaplan changed the title ~~First e2e test~~ e2e, Introduce e2e test: resolving a FQDN entry Dec 5, 2022

kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 5, 2022

AlonaKaplan force-pushed the firt_e2e branch 2 times, most recently from f9a6b1f to 7b3fa53 Compare December 5, 2022 09:07

oshoval reviewed Dec 5, 2022

View reviewed changes

hack/functest.sh Outdated

echo hello world

source ./cluster/cluster.sh

[ "$CI" == "true" ] && dnf install bind-utils

Copy link

Collaborator

oshoval Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-y

AlonaKaplan force-pushed the firt_e2e branch from 7b3fa53 to dfa5670 Compare December 5, 2022 09:53

oshoval reviewed Dec 5, 2022

View reviewed changes

tests/client.go Outdated Show resolved Hide resolved

oshoval reviewed Dec 5, 2022

View reviewed changes

AlonaKaplan force-pushed the firt_e2e branch 2 times, most recently from dfb63de to e8429af Compare December 5, 2022 12:43

oshoval reviewed Dec 6, 2022

View reviewed changes

AlonaKaplan force-pushed the firt_e2e branch from e8429af to 368c4d8 Compare December 6, 2022 11:43

oshoval reviewed Dec 6, 2022

View reviewed changes

AlonaKaplan added 3 commits December 6, 2022 15:28

e2e: Adding first e2e verifying nslookup to a newly created VM interface

c760661

Signed-off-by: Alona Paz <alkaplan@redhat.com>

make vendor output

57e6b83

Signed-off-by: Alona Paz <alkaplan@redhat.com>

Update the functest.sh to invoke the e2e tests

b1847b3

The script also makes sure `nslookup` is installed. Signed-off-by: Alona Paz <alkaplan@redhat.com>

AlonaKaplan force-pushed the firt_e2e branch from 368c4d8 to b1847b3 Compare December 6, 2022 13:31

oshoval approved these changes Dec 6, 2022

View reviewed changes

kubevirt-bot assigned oshoval Dec 6, 2022

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 6, 2022

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2022

kubevirt-bot merged commit 812cfcf into main Dec 6, 2022

AlonaKaplan deleted the firt_e2e branch December 6, 2022 14:02

		var testNamespace string

		var _ = Describe("Virtual Machines Startup", func() {

e2e, Introduce e2e test: resolving a FQDN entry #38

e2e, Introduce e2e test: resolving a FQDN entry #38

Conversation

AlonaKaplan commented Dec 4, 2022 • edited Loading

kubevirt-bot commented Dec 4, 2022

oshoval commented Dec 5, 2022 • edited Loading

oshoval left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oshoval Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

oshoval Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oshoval Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oshoval Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oshoval commented Dec 5, 2022

oshoval Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlonaKaplan commented Dec 5, 2022

oshoval commented Dec 5, 2022

oshoval commented Dec 5, 2022 • edited Loading

oshoval commented Dec 5, 2022

Choose a reason for hiding this comment

oshoval left a comment

Choose a reason for hiding this comment

oshoval commented Dec 5, 2022 • edited Loading

oshoval Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oshoval Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oshoval Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlonaKaplan commented Dec 5, 2022

oshoval commented Dec 5, 2022

AlonaKaplan commented Dec 6, 2022

oshoval commented Dec 6, 2022

oshoval left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oshoval Dec 6, 2022 • edited Loading

AlonaKaplan commented Dec 4, 2022 •

edited

Loading

oshoval commented Dec 5, 2022 •

edited

Loading

oshoval Dec 5, 2022 •

edited

Loading

oshoval Dec 5, 2022 •

edited

Loading

oshoval Dec 5, 2022 •

edited

Loading

oshoval Dec 5, 2022 •

edited

Loading

oshoval Dec 5, 2022 •

edited

Loading

oshoval commented Dec 5, 2022 •

edited

Loading

oshoval commented Dec 5, 2022 •

edited

Loading

oshoval Dec 5, 2022 •

edited

Loading

oshoval Dec 5, 2022 •

edited

Loading

oshoval Dec 5, 2022 •

edited

Loading

oshoval Dec 6, 2022 •

edited

Loading

oshoval Dec 6, 2022 •

edited

Loading