Fix/upgrade manual #207
Merged
Fix/upgrade manual #207
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
mdrahman-suse
force-pushed
the
fix/upgrade_manual
branch
18 times, most recently
from
18d0ebc to
fe15f2f
Compare
mdrahman-suse
force-pushed
the
fix/upgrade_manual
branch
8 times, most recently
from
f228b16 to
e69db6a
Compare
mdrahman-suse
force-pushed
the
fix/upgrade_manual
branch
from
e69db6a to
2e43006
Compare
added 2 commits
January 30, 2025 10:41
mdrahman-suse
force-pushed
the
fix/upgrade_manual
branch
from
520396a to
1b60827
Compare
mdrahman-suse
force-pushed
the
fix/upgrade_manual
branch
from
1b60827 to
2550acb
Compare
fmoral2
reviewed
Jan 31, 2025
VestigeJ
reviewed
Jan 31, 2025
pkg/testcase/daemonset.go
Outdated
| @@ -17,6 +18,12 @@ func TestDaemonset(applyWorkload, deleteWorkload bool) { | |||
| workloadErr = shared.ManageWorkload("apply", "daemonset.yaml") | |||
| Expect(workloadErr).NotTo(HaveOccurred(), "Daemonset manifest not deployed") | |||
| } | |||
|
|
|||
| getDeamonset := "kubectl get pods -n test-daemonset" + | |||
| " --field-selector=status.phase=Running --kubeconfig=" | |||
There was a problem hiding this comment.
is the kubeconfig passed into this command or not? This looks to not be passing it like elsewhere
" --kubeconfig=" + shared.KubeConfigFile)
There was a problem hiding this comment.
It is being passed on the next line
There was a problem hiding this comment.
To add to Justin's comment, for better readability could we have this getDeamonset+shared.KubeConfigFile set like in rest of the commands?
VestigeJ
reviewed
Jan 31, 2025
pkg/testcase/upgrademanual.go
Outdated
| output, err = ms.ManageService(ip, status) | ||
| if output != "" { | ||
| Expect(output).To(ContainSubstring("active "), | ||
| fmt.Sprintf("error checking status %s service for %s node ip: %s", product, nodeType, ip)) |
There was a problem hiding this comment.
shared.LogLevel("error", "") // ?
VestigeJ
reviewed
Jan 31, 2025
pkg/testcase/upgrademanual.go
Outdated
| output, err := ms.ManageService(ip, status) | ||
| if output != "" { | ||
| Expect(output).To(ContainSubstring("active "), | ||
| fmt.Sprintf("error starting %s service for %s node ip: %s", product, nodeType, ip)) |
There was a problem hiding this comment.
shared.LogLevel("error", "") // ?
There was a problem hiding this comment.
Added expect here so no need to log, will update the expect message
VestigeJ
reviewed
Jan 31, 2025
pkg/testcase/upgrademanual.go
Outdated
| Expect(output).To(ContainSubstring("active "), | ||
| fmt.Sprintf("error starting %s service for %s node ip: %s", product, nodeType, ip)) | ||
| } | ||
| Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("error starting %s service on %s", product, ip)) |
There was a problem hiding this comment.
shared.LogLevel("error", "") // ?
There was a problem hiding this comment.
mdrahman-suse
force-pushed
the
fix/upgrade_manual
branch
7 times, most recently
from
1d57851 to
6d7322c
Compare
fmoral2
reviewed
Feb 4, 2025
mdrahman-suse
force-pushed
the
fix/upgrade_manual
branch
4 times, most recently
from
6c2773b to
43a94ff
Compare
pkg/testcase/pod.go
Outdated
| pods, err := shared.GetPods(false) | ||
| g.Expect(err).NotTo(HaveOccurred()) | ||
| g.Expect(pods).NotTo(BeEmpty()) | ||
|
|
||
| res, _ := shared.RunCommandHost(cmd + " --kubeconfig=" + shared.KubeConfigFile) | ||
| if res != "" { | ||
| shared.LogLevel("info", "Pods not Running or Pending: \n%s", res) |
There was a problem hiding this comment.
Waiting for the pod status to become "Running".
| } | ||
|
|
||
| return true | ||
| }, "600s", "10s").Should(BeTrue(), "failed to process pods status") |
There was a problem hiding this comment.
Could we consider rephrasing it to 'Pod is not in the desired state,' since it seems that its the pod's status rather than the processing of it?
mdrahman-suse
force-pushed
the
fix/upgrade_manual
branch
from
43a94ff to
a0c1ae6
Compare
mdrahman-suse
force-pushed
the
fix/upgrade_manual
branch
from
e698756 to
f0d6503
Compare
fmoral2
approved these changes
Feb 5, 2025
ShylajaDevadiga
approved these changes
Feb 5, 2025
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.
Proposed Changes
Types of Changes
Testing
Checklist:
If your PR changes anything on or related to Jenkins, run it pointing to your branch to make sure it's okay.
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
Further Comments