Ingress Upgrade Testing #546
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @agau4779. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
size/L
|
/assign @rramkumar1 |
agau4779
force-pushed
the
upgrade-testing
branch
2 times, most recently
from
8caab2d
to
d9fb8ae
Compare
|
/assign bowei |
pkg/e2e/status.go
Outdated
| ) | ||
|
|
||
| const ( | ||
| configMapName = "status-cm" |
There was a problem hiding this comment.
should we create unique config maps for each run (per framework instance) to avoid potential clashes if we have a messed up run?
There was a problem hiding this comment.
hm, on the guitar test side we need to know the name of the configmap in order to check the conents. @rramkumar1 - thoughts? What does a messed-up run look like in this case?
There was a problem hiding this comment.
@bowei What do you mean by messed up run?
There was a problem hiding this comment.
ignore my comment, after discussion, I don't think it makes sense.
|
/ok-to-test |
agau4779
force-pushed
the
upgrade-testing
branch
from
d9fb8ae
to
445739a
Compare
k8s-ci-robot
removed
the
needs-ok-to-test
agau4779
force-pushed
the
upgrade-testing
branch
from
bc5a761
to
5b014bd
Compare
| // Loop until we successfully update the config map | ||
| for { | ||
| var err error | ||
| sm.cm, err = sm.f.Clientset.Core().ConfigMaps("default").Update(sm.cm) |
There was a problem hiding this comment.
could it a race if we do update rather than patch?
agau4779
force-pushed
the
upgrade-testing
branch
from
5b014bd
to
6587a75
Compare
| options := &e2e.WaitForIngressOptions{ | ||
| ExpectUnreachable: runs == 0, | ||
| } | ||
| ing, err := e2e.WaitForIngress(s, tc.ing, options) |
There was a problem hiding this comment.
I forget -- does this have some delay in the check to avoid excessively spamming when the LB is down?
| needUpdate := true | ||
| // Framework.shutdown() kills this loop. | ||
| for { | ||
| if s.MasterUpgraded() && needUpdate { |
There was a problem hiding this comment.
do this in a follow on--
is it possible to separate out the wait for upgrade logic so it's easier to reuse?
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: agau4779, bowei 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 |
k8s-ci-robot
added
the
approved
Continuation of #392 .