Add RHEL8 and RHEL9 fix for validation webhook and tests for Nutanix #6822

adiantum · 2023-10-16T03:23:35Z

add rhel8 and rhel9 tests for nutanix
add rhel8 and rhel9 template vars
add rhel9 constants
change NutanixMachineConfig validation

eks-distro-bot · 2023-10-16T03:23:47Z

Hi @adiantum. Thanks for your PR.

I'm waiting for a aws member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

- add tests - add rhel8 and rhel9 template vars - add rhel9 constants - change NutanixMachineConfig validation

adiantum · 2023-10-17T22:53:05Z

/assign @abhinavmpandey08

ahreehong · 2023-10-17T23:02:06Z

pkg/api/v1alpha1/nutanixmachineconfig.go

@@ -153,14 +153,6 @@ func validateNutanixMachineConfig(c *NutanixMachineConfig) error {
 		return fmt.Errorf("NutanixMachineConfig: %v", err)
 	}

-	if c.Spec.OSFamily != Ubuntu {


instead of deleting this entire validation, can we do something like
if c.Spec.OSFamily != Ubuntu && c.Spec.OSFamily != RedHat

for ex:

eks-anywhere/pkg/api/v1alpha1/vspheremachineconfig.go

Line 87 in e53af28

if config.Spec.OSFamily != Bottlerocket && config.Spec.OSFamily != Ubuntu && config.Spec.OSFamily != RedHat {

@ahreehong Fixed

Does RedHat OSFamily cover both redhat8 and redhat9

@abhinavmpandey08 Yeah.
https://github.com/aws/eks-anywhere/pull/6822/files#diff-ed18f85e94add304fc61a5b5e588087980e478789e065ae3e8e2ea7a38735a75L27-R31

abhinavmpandey08 · 2023-10-17T23:19:14Z

test/e2e/nutanix_test.go

+func TestNutanixKubernetes128RedHatSimpleFlowWithUUID(t *testing.T) {
+	test := framework.NewClusterE2ETest(
+		t,
+		framework.NewNutanix(t, framework.WithRedHat128NutanixUUID(),
+			framework.WithPrismElementClusterUUID(),
+			framework.WithNutanixSubnetUUID()),
+		framework.WithClusterFiller(api.WithKubernetesVersion(v1alpha1.Kube128)),
+	)
+	runSimpleFlow(test)
+}


I dont think we need separate UUID tests since the images are going to be the same and it's not really adding a lot more coverage. If we really wanted, we can keep just the 128 tests for UUID and remove the other ones.

@abhinavmpandey08 Fixed

abhinavmpandey08 · 2023-10-18T00:55:04Z

/ok-to-test

codecov · 2023-10-19T16:00:29Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (61784f0) 71.99% compared to head (2b7ee88) 72.01%.
Report is 20 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6822      +/-   ##
==========================================
+ Coverage   71.99%   72.01%   +0.02%     
==========================================
  Files         532      532              
  Lines       41597    41641      +44     
==========================================
+ Hits        29947    29988      +41     
- Misses       9978     9980       +2     
- Partials     1672     1673       +1

Files	Coverage Δ
pkg/api/v1alpha1/nutanixmachineconfig.go	`92.96% <100.00%> (+0.05%)`	⬆️

... and 15 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

abhinavmpandey08

Can you add all the new RHEL env vars in this file here: https://github.com/aws/eks-anywhere/blob/main/cmd/integration_test/build/buildspecs/nutanix-test-eks-a-cli.yml#L42

Something like this for all the envs should be good:

    T_NUTANIX_TEMPLATE_NAME_REDHAT_1_24: "nutanix_ci:nutanix_template_rhel_8_1_24"
    ...
    T_NUTANIX_TEMPLATE_NAME_REDHAT_9_1_24: "nutanix_ci:nutanix_template_rhel_9_1_24"
    ...

abhinavmpandey08 · 2023-10-19T15:59:00Z

test/framework/nutanix.go

-	nutanixTemplateNameUbuntu123Var,
-	nutanixTemplateNameUbuntu124Var,
-	nutanixTemplateNameUbuntu125Var,
-	nutanixTemplateNameUbuntu126Var,
-	nutanixTemplateNameUbuntu127Var,
-	nutanixTemplateNameUbuntu128Var,


Can you add these back along with all the new RHEL env vars? These are needed for the tests to run in our CI.

abhinavmpandey08 · 2023-10-19T16:04:03Z

test/framework/nutanix.go

+	nutanixTemplateNameUbuntu126Var  = "T_NUTANIX_TEMPLATE_NAME_UBUNTU_1_26"
+	nutanixTemplateNameUbuntu127Var  = "T_NUTANIX_TEMPLATE_NAME_UBUNTU_1_27"
+	nutanixTemplateNameUbuntu128Var  = "T_NUTANIX_TEMPLATE_NAME_UBUNTU_1_28"
+	nutanixTemplateNameRedHat123Var  = "T_NUTANIX_TEMPLATE_NAME_REDHAT_1_23"


We are removing support for k8s v1.23 so we can remove this

abhinavmpandey08 · 2023-10-19T16:04:23Z

test/framework/nutanix.go

+	nutanixTemplateNameRedHat126Var  = "T_NUTANIX_TEMPLATE_NAME_REDHAT_1_26"
+	nutanixTemplateNameRedHat127Var  = "T_NUTANIX_TEMPLATE_NAME_REDHAT_1_27"
+	nutanixTemplateNameRedHat128Var  = "T_NUTANIX_TEMPLATE_NAME_REDHAT_1_28"
+	nutanixTemplateNameRedHat9123Var = "T_NUTANIX_TEMPLATE_NAME_REDHAT_9_1_23"


This isn't needed anymore either for the same reason

adiantum · 2023-10-19T18:46:03Z

Can you add all the new RHEL env vars in this file here: https://github.com/aws/eks-anywhere/blob/main/cmd/integration_test/build/buildspecs/nutanix-test-eks-a-cli.yml#L42

Something like this for all the envs should be good:
    T_NUTANIX_TEMPLATE_NAME_REDHAT_1_24: "nutanix_ci:nutanix_template_rhel_8_1_24"
    ...
    T_NUTANIX_TEMPLATE_NAME_REDHAT_9_1_24: "nutanix_ci:nutanix_template_rhel_9_1_24"
    ...

Fixed

abhinavmpandey08

/approve
/lgtm

eks-distro-bot · 2023-10-19T18:52:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavmpandey08

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

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

eks-distro-bot added the needs-ok-to-test label Oct 16, 2023

eks-distro-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 16, 2023

Add RHEL8 and RHEL9 tests for Nutanix

8f1c8d5

- add tests - add rhel8 and rhel9 template vars - add rhel9 constants - change NutanixMachineConfig validation

adiantum changed the title ~~Add RHEL8 and RHEL9 tests for Nutanix~~ Add RHEL8 and RHEL9 fix for validation webhook and tests for Nutanix Oct 17, 2023

eks-distro-bot assigned abhinavmpandey08 Oct 17, 2023

ahreehong reviewed Oct 17, 2023

View reviewed changes

abhinavmpandey08 reviewed Oct 17, 2023

View reviewed changes

eks-distro-bot added ok-to-test and removed needs-ok-to-test labels Oct 18, 2023

Fix review comments

53fadf1

ahreehong approved these changes Oct 18, 2023

View reviewed changes

eks-distro-bot assigned ahreehong Oct 18, 2023

eks-distro-bot added the lgtm label Oct 18, 2023

abhinavmpandey08 requested changes Oct 19, 2023

View reviewed changes

eks-distro-bot removed the lgtm label Oct 19, 2023

Fix review comments

2b7ee88

abhinavmpandey08 approved these changes Oct 19, 2023

View reviewed changes

eks-distro-bot added the lgtm label Oct 19, 2023

eks-distro-bot added the approved label Oct 19, 2023

eks-distro-bot merged commit 0d8e817 into aws:main Oct 19, 2023
5 checks passed

adiantum deleted the ntnx-rhel-tests branch October 24, 2023 15:28

abhinavmpandey08 mentioned this pull request Oct 30, 2023

0.18.0 Changelog #6844

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RHEL8 and RHEL9 fix for validation webhook and tests for Nutanix #6822

Add RHEL8 and RHEL9 fix for validation webhook and tests for Nutanix #6822

adiantum commented Oct 16, 2023 •

edited

Loading

eks-distro-bot commented Oct 16, 2023

adiantum commented Oct 17, 2023

ahreehong Oct 17, 2023

adiantum Oct 18, 2023

abhinavmpandey08 Oct 18, 2023

adiantum Oct 18, 2023

abhinavmpandey08 Oct 17, 2023

adiantum Oct 18, 2023

abhinavmpandey08 commented Oct 18, 2023

codecov bot commented Oct 19, 2023 •

edited

Loading

abhinavmpandey08 left a comment

abhinavmpandey08 Oct 19, 2023

adiantum Oct 19, 2023

abhinavmpandey08 Oct 19, 2023

adiantum Oct 19, 2023

abhinavmpandey08 Oct 19, 2023

adiantum Oct 19, 2023

adiantum commented Oct 19, 2023

abhinavmpandey08 left a comment

eks-distro-bot commented Oct 19, 2023

Add RHEL8 and RHEL9 fix for validation webhook and tests for Nutanix #6822

Add RHEL8 and RHEL9 fix for validation webhook and tests for Nutanix #6822

Conversation

adiantum commented Oct 16, 2023 • edited Loading

eks-distro-bot commented Oct 16, 2023

adiantum commented Oct 17, 2023

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

abhinavmpandey08 commented Oct 18, 2023

codecov bot commented Oct 19, 2023 • edited Loading

Codecov Report

abhinavmpandey08 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

Choose a reason for hiding this comment

adiantum commented Oct 19, 2023

abhinavmpandey08 left a comment

Choose a reason for hiding this comment

eks-distro-bot commented Oct 19, 2023

adiantum commented Oct 16, 2023 •

edited

Loading

codecov bot commented Oct 19, 2023 •

edited

Loading