cnf ran:siteconfig-operator:ready-for-final-review: adding 'ocp-75374-75376' test cases #196
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
klaskosk
reviewed
Sep 13, 2024
mpmaruthu
force-pushed
the
cnf-ocp-75376
branch
2 times, most recently
from
6388573
to
6db3323
Compare
klaskosk
reviewed
Sep 19, 2024
Comment on lines
38
to
41
| // SNO represents spoke cluster type as single-node openshift (SNO) cluster. | ||
| SNO = "sno" | ||
| // Standard represents spoke cluster type as multi-node openshift (MNO) cluster. | ||
| Standard = "standard" |
There was a problem hiding this comment.
Could you make the cluster type into a separate type like what HubOperatorName does?
There was a problem hiding this comment.
Okay, updated. Thanks!
| return ranparam.SNO, nil | ||
| } | ||
|
|
||
| return ranparam.Standard, fmt.Errorf("could not determine spoke cluster type") |
There was a problem hiding this comment.
for the error case, could you make it return "" instead of standard?
There was a problem hiding this comment.
Okay, updated. Thanks!
Comment on lines
103
to
132
| By("resetting the clusters app back to the original settings") | ||
| err = gitdetails.SetGitDetailsInArgoCd( | ||
| tsparams.ArgoCdClustersAppName, tsparams.ArgoCdAppDetails[tsparams.ArgoCdClustersAppName], true, false) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to reset clusters app git details") | ||
|
|
||
| // Test step 2 expected results validation. | ||
| By("checking the infra env manifests exists on hub") | ||
| _, err = assisted.PullInfraEnvInstall(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to find spoke infra env manifests") | ||
|
|
||
| By("checking the bare metal host manifests exists on hub") | ||
| _, err = bmh.Pull(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to find spoke bmh manifests") | ||
|
|
||
| By("checking the cluster deployment manifests exists on hub") | ||
| _, err = hive.PullClusterDeployment(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to find spoke cluster deployment manifests") | ||
|
|
||
| By("checking the NM state config manifests exists on hub") | ||
| nmStateConfigList, err = assisted.ListNmStateConfigs(HubAPIClient, RANConfig.Spoke1Name) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to list NM state config manifests") | ||
| Expect(nmStateConfigList).ToNot(BeEmpty(), "Failed to find NM state config manifests") | ||
|
|
||
| By("checking the klusterlet addon config manifests exists on hub") | ||
| _, err = ocm.PullKAC(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to find spoke kac manifests") | ||
|
|
||
| By("checking the agent cluster install manifests exists on hub") | ||
| _, err = assisted.PullAgentClusterInstall(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to find spoke agent cluster install manifests") |
There was a problem hiding this comment.
are these the same between the test cases? maybe they should go into an AfterEach
There was a problem hiding this comment.
Okay, updated. Thanks!
mpmaruthu
force-pushed
the
cnf-ocp-75376
branch
from
6db3323
to
6c94c95
Compare
klaskosk
reviewed
Sep 19, 2024
Comment on lines
98
to
125
| By("checking the infra env manifests removed on hub") | ||
| _, err = assisted.PullInfraEnvInstall(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).To(HaveOccurred(), "Found spoke infra env manifests but expected to be removed") | ||
|
|
||
| By("checking the bare metal host manifests removed on hub") | ||
| _, err = bmh.Pull(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).To(HaveOccurred(), "Found spoke bmh manifests but expected to be removed") | ||
|
|
||
| By("checking the cluster deployment manifests removed on hub") | ||
| _, err = hive.PullClusterDeployment(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).To(HaveOccurred(), "Found spoke cluster deployment manifests but expected to be removed") | ||
|
|
||
| By("checking the NM state config manifests removed on hub") | ||
| nmStateConfigList, err := assisted.ListNmStateConfigs(HubAPIClient, RANConfig.Spoke1Name) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to list NM state config manifests") | ||
| Expect(nmStateConfigList).To(BeEmpty(), "Found spoke NM state config manifests but expected to be removed") | ||
|
|
||
| By("checking the klusterlet addon config manifests removed on hub") | ||
| _, err = ocm.PullKAC(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).To(HaveOccurred(), "Found spoke kac manifests but expected to be removed") | ||
|
|
||
| By("checking the agent cluster install manifests removed on hub") | ||
| _, err = assisted.PullAgentClusterInstall(HubAPIClient, RANConfig.Spoke1Name, RANConfig.Spoke1Name) | ||
| Expect(err).To(HaveOccurred(), "Found spoke ACI manifests but expected to be removed") | ||
|
|
||
| By("verifying installed spoke cluster should still be functional") | ||
| _, err = version.GetOCPVersion(Spoke1APIClient) | ||
| Expect(err).ToNot(HaveOccurred(), "Failed to get OCP version from spoke and verify spoke cluster access") |
There was a problem hiding this comment.
if these are the same between the two cases, they could be moved to a function at the end of this file
There was a problem hiding this comment.
Okay,updated. Thanks!
mpmaruthu
force-pushed
the
cnf-ocp-75376
branch
from
6c94c95
to
165e3a7
Compare
mpmaruthu
force-pushed
the
cnf-ocp-75376
branch
from
165e3a7
to
adfa642
Compare
mpmaruthu
changed the title
mpmaruthu
changed the title
klaskosk
reviewed
Sep 20, 2024
| glog.V(ranparam.LogLevel).Infof("podList[0] matching podLabelSelector is '%v'", | ||
| podList[0].Definition.Name) | ||
|
|
||
| return podList[0].Definition.Name, err |
There was a problem hiding this comment.
This will panic if no pods exist with the label or the list function returns an error
|
|
||
| // ValidateAISpokeClusterInstallCRsRemoved verifies AI spoke cluster install CRs removed and spoke cluster accessible. | ||
| func ValidateAISpokeClusterInstallCRsRemoved() { | ||
| By("checking the infra env manifests removed on hub", func() { |
There was a problem hiding this comment.
for consistency, could you please switch this PR to use the form of By without the func() {}?
Comment on lines
+91
to
+93
| Expect(err).ToNot(HaveOccurred(), "Failed to find default AI cluster level templates config map") | ||
|
|
||
| if err == nil { |
There was a problem hiding this comment.
since you assert the error didn't occur, it is always nil so the if statement is unnecessary. Is it a test failure if the cm does not exist?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@mcornea , @klaskosk , Please review this PR when you have a time.
cc: @yliu127 , @dgonyier
Thanks.
/Peri M
Adding siteconfig-operator test case - OCP-75374
Adding siteconfig-operator test case - OCP-75376
Sample test configuration for your reference:
http://registry.kni-qe-0.lab.eng.rdu2.redhat.com:3000/kni-qe/ztp-site-configs/src/8549392f6abc0f776a4d35e74e13d4e40cdb11e2/siteconfig/ztp-siteconfigoperator-test/detach-ai-sno/kustomization.yaml#L4
Manual test logs:-
Test case 1: https://docs.google.com/document/d/1ihBMjxsZmi2RRX90f5a--vmqzb0hDUxWQIlK4t8Wugk/edit?usp=sharing
Test case 2: https://docs.google.com/document/d/1FsTuWvS0UGaCujsfy5UQRJIlU69T2TdbGJ6Jt6OFANA/edit?usp=sharing