Go vunlncheck fix #646
Go vunlncheck fix #646
Conversation
* Increase test sleep to counter flaky create
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nckturner 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
approved
cmd/aws-iam-authenticator/server.go
Outdated
| @@ -52,15 +53,16 @@ var serverCmd = &cobra.Command{ | |||
| var err error | |||
| fmt.Printf("Authenticator Version: %q, %q\n", pkg.Version, pkg.CommitID) | |||
| metrics.InitMetrics(prometheus.DefaultRegisterer) | |||
| stopCh := signals.SetupSignalHandler() | |||
| cmd.Context() | |||
There was a problem hiding this comment.
@micahhausler what are we doing with this? https://github.com/spf13/cobra/blob/main/command.go#L257-L266
| return | ||
| default: | ||
| watcher, err := ms.configMap.Watch(context.TODO(), metav1.ListOptions{ | ||
| watcher, err := ms.configMap.Watch(context.Background(), metav1.ListOptions{ |
There was a problem hiding this comment.
@micahhausler originally you had this set to the same context that was passed in to startLoadConfigMap(), but this was causing the context to be closed when the watch call failed. I'm not sure what the right thing to do here is though.
nckturner
force-pushed
the
go-vunlncheck-fix
branch
6 times, most recently
from
c938a74 to
b6ff237
Compare
* Context for configmap watch needs to be created from Background(), otherwise watch failure will cause authenticator to crash. * Add some logging. * Remove unnecessary references to stopCh.
nckturner
force-pushed
the
go-vunlncheck-fix
branch
from
b6ff237 to
baea9c0
Compare
|
Need this PR for integration tests: kubernetes/test-infra#31295 |
|
/test pull-aws-iam-authenticator-integration |
|
/retest |
|
/test pull-aws-iam-authenticator-integration I added dind to the job config to fix the following error: Not sure why its still failing on that error. Trying to run again, maybe needed more time to propagate the job config. |
|
@nckturner: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. 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. |
What this PR does / why we need it: