test/e2e: check MC deployed on every node #446
Conversation
openshift-ci-robot
added
approved
runcom
force-pushed
the
fix-test-mcdeployed
branch
from
c400092
to
787b0f9
Compare
|
Unit flake #417 /retest |
test/e2e/mcd_test.go
Outdated
| continue | ||
| } | ||
| for _, line := range strings.Split(string(res), "\n") { | ||
| if strings.Contains(line, "completed update for config "+newMCName) { |
There was a problem hiding this comment.
Seems odd to parse the log files for this; why not verify the currentConfig annotation?
There was a problem hiding this comment.
Or for that matter, watch the worker machineconfigpool, which is doing just that.
There was a problem hiding this comment.
changed to watch for status changes for the pool (even if now it's intrinsically racy cause we may somehow be missing some status, for instance, we may very well miss the Updating status, or check the previous Updated status thinking all is good if we don't check Updating and so on). It was odd to check logs of the MCDs but to me it was more robust than checking status. Maybe checking currentConfig may instead be better. We may be using channels for syncronization for status updates but that complicates the things as well so let's just fix the test now making sure every node is updated
There was a problem hiding this comment.
ok changed to check currentConfig instead. (the pool status check is here though if you want to take a look c40af43)
runcom
force-pushed
the
fix-test-mcdeployed
branch
2 times, most recently
from
c40af43
to
0f49594
Compare
TestMCDeployed wasn't checking that the MC was indeed deployed on every node (worker). Fix that by checking every node. The test is going to take a little longer but it's more robust as well. Signed-off-by: Antonio Murdaca <runcom@linux.com>
runcom
force-pushed
the
fix-test-mcdeployed
branch
from
0f49594
to
64b2987
Compare
|
Haproxy flake /retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, runcom 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:
Approvers can indicate their approval by writing |
Signed-off-by: Antonio Murdaca runcom@linux.com
- What I did
TestMCDeployed wasn't checking that the MC was indeed deployed on every
node (worker). Fix that by checking every node. The test is going to
take a little longer but it's more robust as well.
- How to verify it
run e2e
- Description for the changelog