-
Notifications
You must be signed in to change notification settings - Fork 392
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
test/e2e: add MC smoke test #387
Conversation
Far away from being the right tests we need to test this whole workload but it's a start. Just making sure we can see the daemon (1) actually converge to the new config. (Next step would be to deploy a pod with an hostPath referencing an MC file, exec into this pod and verify the file is rightly there) Signed-off-by: Antonio Murdaca <runcom@linux.com>
/retest |
so this is green but I can't check if the test actually passed (like in |
LabelSelector: labels.SelectorFromSet(labels.Set{"k8s-app": "machine-config-daemon"}).String(), | ||
} | ||
|
||
err = wait.Poll(3*time.Second, 5*time.Minute, func() (bool, error) { |
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.
Minor: hmm, so this is going over every log of every pod? Could we make this smarter by only going over pods on worker nodes?
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.
I could do that yeah
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.
It's also only the MCD pods right?
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.
yeah, should be #387 (diff)
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.
Yeah, true. So every log of every MCD pod, rather than just worker MCD pods. Guess if it's not hard to select for only worker nodes, it'd be a nice addition, otherwise meh!
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.
I'm checking that, but MCDs aren't referencing worker or master explicitly so it's gonna be a great dance to get worker nodes and then match on names, what do you think?
It's not intuitive, but it's in there. Click on "Details", then "artifacts", then go to
Anyway, this looks good to me overall (woohoo more tests!). Will let others have a look. |
Yeah, totally fine as is! /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlebon, 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 |
let's try to populate the prow interface with actual tests though so we don't need to do that everytime |
/retest |
1 similar comment
/retest |
Far away from being the right tests we need to test this whole workload
but it's a start. Just making sure we can see the daemon (1) actually
converge to the new config. (Next step would be to deploy a pod with
an hostPath referencing an MC file, exec into this pod and verify
the file is rightly there)
Signed-off-by: Antonio Murdaca runcom@linux.com