Closed rancher-max closed 9 months ago
have u tested some of those tests from version template? since we are changing stuff on template and signature, it would be good die for sure
please before merge post in here on the PR description 2 successfull jobs from jenkins pointing to your branch:
Sent you links to jobs directly! Should be ready for re-review
Overall LGTM 👍 . Suggestion if we can/want to rename the variable
DEPLOY_WORKLOAD
toAPPLY_WORKLOAD
and make similar changes to thedeployWorkload
variable name toapplyWorkload
to be consistent with its functionality
after comment addressed please post a screenshot of any bump version working .
This is from: go test -timeout=30m -v -tags=versionbump ./entrypoint/versionbump/... -cmd "/var/lib/rancher/k3s/data/current/bin/cni, k3s -v" -expectedValue "v1.3.0-k3s1, v1.27" -testCase "TestServiceClusterIP" -applyWorkload=true -deleteWorkload=true -workloadName="bandwidth-annotations.yaml"
Proposed Changes
This adds another variable to the functions that deal with workloads. This new variable controls whether or not to apply the workload. Without this, we could have false positives in cases where we want tests that had previously deployed a workload to ensure that workload still exists.
These changes also brought to light a flag,
deployWorkload
, that is used in version bump tests. This actually was more of adeleteWorkload
flag, but now we have both deploy and delete as part of these changes.Types of Changes
Fix and refactor
Testing
Checklist:
If your PR changes anything on or related to Jenkins, run it pointing to your branch to make sure it's okay. Jenkins jobs run for SUC Upgrade and etcd version bump available upon request.
Verify code lint; we should not have errors.
Update the documentation if needed.
Update makefile and docker run if adding new tests.
Run your tests at least 4 times with all configurations needed and possible.
If needed test with different os types.
Linked Issues
Hackweek!
Further Comments
In the case of the DNS tests, the workload is just a pod. Therefore, it should be deleted during upgrade, so I am still applying the workload post upgrade.
I have the commits separated out to make review easier for where things changed,