-
Notifications
You must be signed in to change notification settings - Fork 27
Update validation logic #438
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
Update validation logic #438
Conversation
rancher-max
left a comment
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.
Overall I like this change. My only concern is for regressions of any functions that were previously using VerifyClusterPods and expecting that everything from the previous implementation is there now have a changed functionality. There may be a few places missed that were doing this -- actions/provisioning/permutations/permutations.go and actions/workloads/verify.go are two examples in my brief search that don't have the new VerifyClusterDeployments added but are still using VerifyClusterPods.
1597b4e
1597b4e to
1be12a4
Compare
|
ExtKubeconfigTestSuite SetupSuite TestSuites above were modified. TestSuites below use modified code from this PR. TestAutoScalingDown |
|
@rancher-max The old cluster permutations is no longer used as far as I know. The changes I made simply divide the 2 checks so you can explicitly choose what you want to check. I could add a deployment check to the verifyWorkloads if you prefer that. I also did a quick search and other then a few deprecated tests it should be added to all relevant places. It also might be worth mentioning that all the code that got moved to the VerifyClustersDeployments func was added by me fairly recently. |
1be12a4 to
6dc43ee
Compare
|
Sounds good! Don't let my review block, was just calling out what I saw quickly 👍 |
slickwarren
left a comment
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.
some small nits, overall lgtm.
6dc43ee to
c21aa83
Compare
c21aa83 to
7ebaf1a
Compare
Modularize verify logic Previously we added a check to the VerifyClusterPods that verifies certain required deployments spin up (SUC, fleet, rancher-webhook, cluster-agent) Update template tests template test fix linter fix Update verify.go Update verify.go
7ebaf1a to
5c301ec
Compare
|
ExtKubeconfigTestSuite SetupSuite TestSuites above were modified. TestSuites below use modified code from this PR. TestAutoScalingDown |
Modularize verify logic
Previously we added a check to the VerifyClusterPods that verifies certain required deployments spin up (SUC, fleet, rancher-webhook, cluster-agent).
Changes: