🌱 Add e2e external inspection test #1382

lentzi90 · 2023-10-09T10:13:13Z

What this PR does / why we need it:

Adds an e2e test for external inspection.
The test simply creates a BMH with annotations for disabling inspection and for adding hardware details. Then it checks that the BMH becomes available without going through inspection.

This also adds cert-manager images to the e2e config so that if they happen to be available locally, then they can be loaded into the kind cluster (if used). It helps save some time when running the test repeatedly.

Minor details:

Don't exit on e2e test failure, gather artifacts/logs first
Destroy the network before trying to undefine it
Wait for namespace deletion between e2e tests since we are using the
same VM for the BMHs in multiple tests

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1370

lentzi90 · 2023-10-09T10:15:11Z

test/e2e/external_inspection_test.go

+	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
+)
+
+const hardwareDetails = `{"cpu":{"arch":"x86_64","count":2,"flags":["3dnowprefetch","abm","adx","aes","apic","arat","arch_capabilities","avx","avx2","avx_vnni","bmi1","bmi2","clflush","clflushopt","clwb","cmov","constant_tsc","cpuid","cpuid_fault","cx16","cx8","de","ept","ept_ad","erms","f16c","flexpriority","fma","fpu","fsgsbase","fsrm","fxsr","gfni","hypervisor","ibpb","ibrs","ibrs_enhanced","invpcid","lahf_lm","lm","mca","mce","md_clear","mmx","movbe","movdir64b","movdiri","msr","mtrr","nopl","nx","ospke","pae","pat","pclmulqdq","pdpe1gb","pge","pku","pni","popcnt","pse","pse36","rdpid","rdrand","rdseed","rdtscp","rep_good","sep","serialize","sha_ni","smap","smep","ss","ssbd","sse","sse2","sse4_1","sse4_2","ssse3","stibp","syscall","tpr_shadow","tsc","tsc_adjust","tsc_deadline_timer","tsc_known_freq","umip","vaes","vme","vmx","vnmi","vpclmulqdq","vpid","waitpkg","x2apic","xgetbv1","xsave","xsavec","xsaveopt","xsaves","xtopology"],"model":"12th Gen Intel(R) Core(TM) i9-12900H"},"firmware":{"bios":{"date":"04/01/2014","vendor":"SeaBIOS","version":"1.15.0-1"}},"hostname":"localhost.localdomain","nics":[{"ip":"192.168.222.122","mac":"00:60:2f:31:81:01","model":"0x1af4 0x0001","name":"enp1s0","pxe":true},{"ip":"fe80::570a:edf2:a3a7:4eb8%enp1s0","mac":"00:60:2f:31:81:01","model":"0x1af4 0x0001","name":"enp1s0","pxe":true}],"ramMebibytes":4096,"storage":[{"alternateNames":["/dev/vda","/dev/disk/by-path/pci-0000:04:00.0"],"name":"/dev/disk/by-path/pci-0000:04:00.0","rotational":true,"sizeBytes":21474836480,"type":"HDD","vendor":"0x1af4"}],"systemVendor":{"manufacturer":"QEMU","productName":"Standard PC (Q35 + ICH9, 2009)"}}`


This was extracted from a BMH that I created manually, by running:

kubectl get bmh test -o jsonpath="{.status.hardware}"

I'm sure it could be formatted properly still?

You mean like this?

Suggested change

const hardwareDetails = `{"cpu":{"arch":"x86_64","count":2,"flags":["3dnowprefetch","abm","adx","aes","apic","arat","arch_capabilities","avx","avx2","avx_vnni","bmi1","bmi2","clflush","clflushopt","clwb","cmov","constant_tsc","cpuid","cpuid_fault","cx16","cx8","de","ept","ept_ad","erms","f16c","flexpriority","fma","fpu","fsgsbase","fsrm","fxsr","gfni","hypervisor","ibpb","ibrs","ibrs_enhanced","invpcid","lahf_lm","lm","mca","mce","md_clear","mmx","movbe","movdir64b","movdiri","msr","mtrr","nopl","nx","ospke","pae","pat","pclmulqdq","pdpe1gb","pge","pku","pni","popcnt","pse","pse36","rdpid","rdrand","rdseed","rdtscp","rep_good","sep","serialize","sha_ni","smap","smep","ss","ssbd","sse","sse2","sse4_1","sse4_2","ssse3","stibp","syscall","tpr_shadow","tsc","tsc_adjust","tsc_deadline_timer","tsc_known_freq","umip","vaes","vme","vmx","vnmi","vpclmulqdq","vpid","waitpkg","x2apic","xgetbv1","xsave","xsavec","xsaveopt","xsaves","xtopology"],"model":"12th Gen Intel(R) Core(TM) i9-12900H"},"firmware":{"bios":{"date":"04/01/2014","vendor":"SeaBIOS","version":"1.15.0-1"}},"hostname":"localhost.localdomain","nics":[{"ip":"192.168.222.122","mac":"00:60:2f:31:81:01","model":"0x1af4 0x0001","name":"enp1s0","pxe":true},{"ip":"fe80::570a:edf2:a3a7:4eb8%enp1s0","mac":"00:60:2f:31:81:01","model":"0x1af4 0x0001","name":"enp1s0","pxe":true}],"ramMebibytes":4096,"storage":[{"alternateNames":["/dev/vda","/dev/disk/by-path/pci-0000:04:00.0"],"name":"/dev/disk/by-path/pci-0000:04:00.0","rotational":true,"sizeBytes":21474836480,"type":"HDD","vendor":"0x1af4"}],"systemVendor":{"manufacturer":"QEMU","productName":"Standard PC (Q35 + ICH9, 2009)"}}`

const hardwareDetails = `

{

"cpu": {

"arch": "x86_64",

"count": 2,

"flags": [

"3dnowprefetch",

"abm",

"adx",

"aes",

"apic",

"arat",

"arch_capabilities",

"avx",

"avx2",

"avx_vnni",

"bmi1",

"bmi2",

"clflush",

"clflushopt",

"clwb",

"cmov",

"constant_tsc",

"cpuid",

"cpuid_fault",

"cx16",

"cx8",

"de",

"ept",

"ept_ad",

"erms",

"f16c",

"flexpriority",

"fma",

"fpu",

"fsgsbase",

"fsrm",

"fxsr",

"gfni",

"hypervisor",

"ibpb",

"ibrs",

"ibrs_enhanced",

"invpcid",

"lahf_lm",

"lm",

"mca",

"mce",

"md_clear",

"mmx",

"movbe",

"movdir64b",

"movdiri",

"msr",

"mtrr",

"nopl",

"nx",

"ospke",

"pae",

"pat",

"pclmulqdq",

"pdpe1gb",

"pge",

"pku",

"pni",

"popcnt",

"pse",

"pse36",

"rdpid",

"rdrand",

"rdseed",

"rdtscp",

"rep_good",

"sep",

"serialize",

"sha_ni",

"smap",

"smep",

"ss",

"ssbd",

"sse",

"sse2",

"sse4_1",

"sse4_2",

"ssse3",

"stibp",

"syscall",

"tpr_shadow",

"tsc",

"tsc_adjust",

"tsc_deadline_timer",

"tsc_known_freq",

"umip",

"vaes",

"vme",

"vmx",

"vnmi",

"vpclmulqdq",

"vpid",

"waitpkg",

"x2apic",

"xgetbv1",

"xsave",

"xsavec",

"xsaveopt",

"xsaves",

"xtopology"

],

"model": "12th Gen Intel(R) Core(TM) i9-12900H"

},

"firmware": {

"bios": {

"date": "04/01/2014",

"vendor": "SeaBIOS",

"version": "1.15.0-1"

}

},

"hostname": "localhost.localdomain",

"nics": [

{

"ip": "192.168.222.122",

"mac": "00:60:2f:31:81:01",

"model": "0x1af4 0x0001",

"name": "enp1s0",

"pxe": true

},

{

"ip": "fe80::570a:edf2:a3a7:4eb8%enp1s0",

"mac": "00:60:2f:31:81:01",

"model": "0x1af4 0x0001",

"name": "enp1s0",

"pxe": true

}

],

"ramMebibytes": 4096,

"storage": [

{

"alternateNames": [

"/dev/vda",

"/dev/disk/by-path/pci-0000:04:00.0"

],

"name": "/dev/disk/by-path/pci-0000:04:00.0",

"rotational": true,

"sizeBytes": 21474836480,

"type": "HDD",

"vendor": "0x1af4"

}

],

"systemVendor": {

"manufacturer": "QEMU",

"productName": "Standard PC (Q35 + ICH9, 2009)"

}

}

`

😬
I'm not really a fan of this but I can accept it.

It does get really long. Could it be put into separate file so it isn't so invasive? It would also make it easier to update if needed.

edit: I see you changed it already. Its fine as is :)

You mean keep it formatted like it is now, or revert to single line?
I'm now wondering also if we should put it in a separate file 🤔
Perhaps that would be the best

lentzi90 · 2023-10-09T10:16:02Z

/metal3-bmo-e2e-test

test/e2e/external_inspection_test.go

lentzi90 · 2023-10-09T11:16:21Z

test/go.mod

@@ -128,3 +129,5 @@ require (
 replace sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.5.0

 replace github.com/metal3-io/baremetal-operator => ../
+
+replace github.com/metal3-io/baremetal-operator/apis => ../apis


Would love input on this. We do the same in the root go.mod so I think this is the correct way.
At the top of the file we require BMO v0.3.1. This old version did not have the annotations that I used in the test. By adding this replacement, we simply use the latest BMO API, not a specific release.

Yes, correct but IMO we should also keep the go.mod up to date as well with latest release versions so as not to confuse the user. We should perhaps do that in all the go modules.

lentzi90 · 2023-10-09T11:17:02Z

/metal3-bmo-e2e-test

lentzi90 · 2023-10-09T11:33:41Z

Ok we got a timeout for BMH going from registering to inspecting. I will increase the timeout for it since it seems to be a flake, and increasing it would avoid triggering the failure.

lentzi90 · 2023-10-09T11:46:25Z

/metal3-bmo-e2e-test

lentzi90 · 2023-10-09T12:04:58Z

Hmm same timeout again... now I suspect there must be something wrong and probably depending on what order the tests are run in. I will investigate.
/hold

hardys · 2023-10-10T07:09:49Z

Hmm same timeout again... now I suspect there must be something wrong and probably depending on what order the tests are run in. I will investigate.
/hold

Can we view the BMO logs from the CI run? I took a look at the console output but couldn't see the logs in the Jenkins console

lentzi90 · 2023-10-10T07:20:52Z

Hmm same timeout again... now I suspect there must be something wrong and probably depending on what order the tests are run in. I will investigate.
/hold

Can we view the BMO logs from the CI run? I took a look at the console output but couldn't see the logs in the Jenkins console

Normally there should be a artifacts.tar.gz that can be downloaded to find the logs. For some reason this is not available on this run. I will investigate this also.

What I have found so far is that the issue is probably that we do not wait properly for the previous BMH to be deleted before we start on the next test and create a new. This is causing registration errors since there is already a node with conflicting MAC.

lentzi90 · 2023-10-10T08:03:28Z

/metal3-bmo-e2e-test

kashifest · 2023-10-10T08:20:27Z

/test-ubuntu-integration-main
/test-centos-e2e-integration-main

lentzi90 · 2023-10-10T08:33:49Z

Lost connection to the jenkins worker...
/metal3-bmo-e2e-test

kashifest · 2023-10-10T08:42:47Z

Following tests are unnrelated to the PR , but for testing the trigger itself, sorry for spamming
/test-ubuntu-integration-release-1.5
/test-centos-e2e-integration-release-1.5

kashifest · 2023-10-10T08:43:21Z

/test-ubuntu-integration-release-0.4
/test-centos-e2e-integration-release-0.4

kashifest · 2023-10-10T08:51:15Z

/test-ubuntu-integration-release-1-5
/test-centos-e2e-integration-release-1-5

lentzi90 · 2023-10-10T09:18:54Z

Artifacts are now properly collected also after test failures 🎉
I will need to adjust the waiting deadlines. Just waiting for @kashifest 's tests to finish before I push

kashifest · 2023-10-10T09:32:46Z

Artifacts are now properly collected also after test failures 🎉 I will need to adjust the waiting deadlines. Just waiting for @kashifest 's tests to finish before I push

No need to wait, I have got what I wanted.

lentzi90 · 2023-10-10T09:34:09Z

/metal3-bmo-e2e-test

lentzi90 · 2023-10-10T09:52:08Z

Successful job!
https://jenkins.nordix.org/job/metal3-bmo-e2e-test-pull/11/
I'm going to trigger it one more time to reduce risk of flakiness.
/metal3-bmo-e2e-test

lentzi90 · 2023-10-10T10:09:12Z

Another success: https://jenkins.nordix.org/job/metal3-bmo-e2e-test-pull/12
Just noticed that I have a typo in a comment... will fix that and push again. With that I think this is good to go 🙂

lentzi90 · 2023-10-10T10:15:24Z

/metal3-bmo-e2e-test
/test-centos-e2e-integration-main
/test-ubuntu-integration-main

lentzi90 · 2023-10-10T10:26:37Z

Connection issues with the jenkins worker again...
/metal3-bmo-e2e-test

kashifest · 2023-10-10T10:41:15Z

/test-centos-e2e-integration-main
/test-ubuntu-integration-main

kashifest · 2023-10-10T10:44:42Z

sorry it seems the test was running already, but for some reason my ui view didnt show it.

lentzi90 · 2023-10-10T11:34:30Z

/hold cancel

mboukhalfa

lgtm

test/e2e/config/fixture.yaml

lentzi90 · 2023-10-12T09:14:28Z

/cc @kashifest @tuminoid

hack/ci-e2e.sh

tuminoid · 2023-10-13T04:55:57Z

test/e2e/external_inspection_test.go

+	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
+)
+
+const hardwareDetails = `{"cpu":{"arch":"x86_64","count":2,"flags":["3dnowprefetch","abm","adx","aes","apic","arat","arch_capabilities","avx","avx2","avx_vnni","bmi1","bmi2","clflush","clflushopt","clwb","cmov","constant_tsc","cpuid","cpuid_fault","cx16","cx8","de","ept","ept_ad","erms","f16c","flexpriority","fma","fpu","fsgsbase","fsrm","fxsr","gfni","hypervisor","ibpb","ibrs","ibrs_enhanced","invpcid","lahf_lm","lm","mca","mce","md_clear","mmx","movbe","movdir64b","movdiri","msr","mtrr","nopl","nx","ospke","pae","pat","pclmulqdq","pdpe1gb","pge","pku","pni","popcnt","pse","pse36","rdpid","rdrand","rdseed","rdtscp","rep_good","sep","serialize","sha_ni","smap","smep","ss","ssbd","sse","sse2","sse4_1","sse4_2","ssse3","stibp","syscall","tpr_shadow","tsc","tsc_adjust","tsc_deadline_timer","tsc_known_freq","umip","vaes","vme","vmx","vnmi","vpclmulqdq","vpid","waitpkg","x2apic","xgetbv1","xsave","xsavec","xsaveopt","xsaves","xtopology"],"model":"12th Gen Intel(R) Core(TM) i9-12900H"},"firmware":{"bios":{"date":"04/01/2014","vendor":"SeaBIOS","version":"1.15.0-1"}},"hostname":"localhost.localdomain","nics":[{"ip":"192.168.222.122","mac":"00:60:2f:31:81:01","model":"0x1af4 0x0001","name":"enp1s0","pxe":true},{"ip":"fe80::570a:edf2:a3a7:4eb8%enp1s0","mac":"00:60:2f:31:81:01","model":"0x1af4 0x0001","name":"enp1s0","pxe":true}],"ramMebibytes":4096,"storage":[{"alternateNames":["/dev/vda","/dev/disk/by-path/pci-0000:04:00.0"],"name":"/dev/disk/by-path/pci-0000:04:00.0","rotational":true,"sizeBytes":21474836480,"type":"HDD","vendor":"0x1af4"}],"systemVendor":{"manufacturer":"QEMU","productName":"Standard PC (Q35 + ICH9, 2009)"}}`


I'm sure it could be formatted properly still?

hack/ci-e2e.sh

.gitignore

The test simply creates a BMH with annotations for disabling inspection and for adding hardware details. Then it checks that the BMH becomes available without going through inspection. This also adds cert-manager images to the e2e config so that if they happen to be available locally, then they can be loaded into the kind cluster (if used). It helps save some time when running the test repetedly. Minor details: - Don't exit on e2e test failure, gather artifacts/logs first - Destroy the network before trying to undefine it - Wait for namespace deletion between e2e tests since we are using the same VM for the BMHs in multiple tests

lentzi90 · 2023-10-13T05:41:45Z

/metal3-bmo-e2e-test

lentzi90 · 2023-10-13T06:28:48Z

/test-centos-e2e-integration-main
/test-ubuntu-integration-main

kashifest

/approve

kashifest · 2023-10-13T06:41:54Z

test/go.mod

@@ -128,3 +129,5 @@ require (
 replace sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.5.0

 replace github.com/metal3-io/baremetal-operator => ../
+
+replace github.com/metal3-io/baremetal-operator/apis => ../apis


Yes, correct but IMO we should also keep the go.mod up to date as well with latest release versions so as not to confuse the user. We should perhaps do that in all the go modules.

hardys · 2023-10-13T09:10:11Z

lgtm

Rozzii

/lgtm

metal3-io-bot · 2023-10-13T12:42:29Z

@Rozzii: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

metal3-io-bot · 2023-10-13T12:42:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kashifest, Rozzii

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~~ [kashifest]

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

tuminoid

/lgtm

Thanks again for good work on BMO e2e!

metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 9, 2023

lentzi90 commented Oct 9, 2023

View reviewed changes

hardys reviewed Oct 9, 2023

View reviewed changes

test/e2e/external_inspection_test.go Show resolved Hide resolved

lentzi90 force-pushed the lentzi90/e2e-external-inspection branch from 9ceb8d7 to f97254f Compare October 9, 2023 11:13

lentzi90 commented Oct 9, 2023

View reviewed changes

lentzi90 force-pushed the lentzi90/e2e-external-inspection branch from f97254f to 9865680 Compare October 9, 2023 11:34

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 9, 2023

lentzi90 force-pushed the lentzi90/e2e-external-inspection branch from 9865680 to a7d4f27 Compare October 10, 2023 07:57

lentzi90 force-pushed the lentzi90/e2e-external-inspection branch from a7d4f27 to 2024ae7 Compare October 10, 2023 09:33

lentzi90 force-pushed the lentzi90/e2e-external-inspection branch from 2024ae7 to a0f7f1b Compare October 10, 2023 10:14

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 10, 2023

lentzi90 mentioned this pull request Oct 10, 2023

BMH with good BMC credentials and registration error stuck deleting #1385

Closed

mboukhalfa reviewed Oct 11, 2023

View reviewed changes

test/e2e/config/fixture.yaml Show resolved Hide resolved

metal3-io-bot requested review from kashifest and tuminoid October 12, 2023 09:14

metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Oct 12, 2023

tuminoid reviewed Oct 13, 2023

View reviewed changes

lentzi90 force-pushed the lentzi90/e2e-external-inspection branch from a0f7f1b to 181d95c Compare October 13, 2023 05:40

metal3-io-bot removed the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Oct 13, 2023

lentzi90 requested a review from tuminoid October 13, 2023 06:29

kashifest reviewed Oct 13, 2023

View reviewed changes

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 13, 2023

Rozzii approved these changes Oct 13, 2023

View reviewed changes

tuminoid reviewed Oct 13, 2023

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2023

metal3-io-bot merged commit 7247b42 into metal3-io:main Oct 13, 2023
12 checks passed

lentzi90 deleted the lentzi90/e2e-external-inspection branch October 13, 2023 18:19

hardys mentioned this pull request Oct 24, 2023

🌱 Re-inspection by annotation test added #1402

Merged

🌱 Add e2e external inspection test #1382

🌱 Add e2e external inspection test #1382

Conversation

lentzi90 commented Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuminoid Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lentzi90 commented Oct 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lentzi90 commented Oct 9, 2023

lentzi90 commented Oct 9, 2023

lentzi90 commented Oct 9, 2023

lentzi90 commented Oct 9, 2023

hardys commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

kashifest commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

kashifest commented Oct 10, 2023

kashifest commented Oct 10, 2023

kashifest commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

kashifest commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

kashifest commented Oct 10, 2023

kashifest commented Oct 10, 2023

lentzi90 commented Oct 10, 2023

mboukhalfa left a comment

Choose a reason for hiding this comment

lentzi90 commented Oct 12, 2023

Choose a reason for hiding this comment

lentzi90 commented Oct 13, 2023

lentzi90 commented Oct 13, 2023

kashifest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hardys commented Oct 13, 2023

Rozzii left a comment

Choose a reason for hiding this comment

metal3-io-bot commented Oct 13, 2023

metal3-io-bot commented Oct 13, 2023

tuminoid left a comment

Choose a reason for hiding this comment

lentzi90 commented Oct 9, 2023 •

edited

Loading

tuminoid Oct 13, 2023 •

edited

Loading