Fix providerless kubelet startup #93607

liggitt · 2020-07-31T16:31:45Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
Follow up from #93579

That PR made the registration of the azure flags conditional (excluded in providerless builds), but the kubelet assumed the flag would be available when registering credential provider flags.

Does this PR introduce a user-facing change?:

NONE

/cc @BenTheElder @aojea

BenTheElder · 2020-07-31T16:36:41Z

/lgtm
Thanks Jordan.
We should get up a smoke test periodic or something

liggitt · 2020-07-31T17:29:39Z

node e2e flake on cpu resources

Node didn't have enough resource

/retest

SergeyKanzhelev · 2020-07-31T19:29:45Z

cmd/kubelet/app/options/globalflags.go

@@ -84,9 +84,7 @@ func addCredentialProviderFlags(fs *pflag.FlagSet) {
 	global := pflag.CommandLine


nit: no need to get global here and pass to addLegacyCloudProviderCredentialProviderFlags . It can be taken when needed.

cmd/kubelet/app/options/globalflags_providers.go

liggitt · 2020-07-31T19:38:49Z

/retest

k8s-ci-robot · 2020-07-31T19:51:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt

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:

~~cmd/kubelet/OWNERS~~ [liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

liggitt · 2020-07-31T19:51:19Z

oops, pushed a commit meant for #93605 ... fixed

BenTheElder · 2020-07-31T19:52:25Z

/lgtm

aojea · 2020-07-31T23:04:22Z

/retest

dims · 2020-08-01T02:36:00Z

/retest

BenTheElder · 2020-08-01T06:46:25Z

/retest

aojea · 2020-08-01T09:47:36Z

hmm
/retest
this is the second time I see the same failure, publishing-bot`

liggitt · 2020-08-01T16:19:47Z

verify is failing on #93623, wait until that is resolved to retest

/hold

liggitt · 2020-08-01T21:16:50Z

/retest
/hold cancel

andrewsykim · 2020-08-02T14:46:29Z

I'm not sure why the AKS e2es would start failing since they shouldn't use providerless builds but ran into this issue in another PR and this seems possibly related 🤔

 2020/08/02 14:22:39 main.go:312: Something went wrong: error creating deployer: failed to get azure credentials: error parsing credentials file /etc/azure-cred/credentials (1, 59): parsing error: no value can start with C
+ EXIT_VALUE=1
+ set +o xtrace
Cleaning up after docker in docker.

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/93582/pull-kubernetes-e2e-aks-engine-azure/1289928857326981121#1:build-log.txt%3A15

BenTheElder · 2020-08-02T16:21:55Z

that tag is not set by default so they'd have to be opting into it somehow

it also doesn't relate to how credentials are parsed, only how a flag is registered (and only if the tag is set).

BenTheElder · 2020-08-02T16:22:38Z

@andrewsykim that log message is from kubetest, not kubelet:

+ kubetest --test --up --down --build=quick --dump=/logs/artifacts --provider=skeleton --deployment=aksengine --aksengine-agentpoolcount=2 --aksengine-admin-username=azureuser --aksengine-creds=/etc/azure-cred/credentials --aksengine-orchestratorRelease=1.18 --aksengine-mastervmsize=Standard_DS2_v2 --aksengine-agentvmsize=Standard_DS2_v2 --aksengine-deploy-custom-k8s --aksengine-location=westus2 --aksengine-public-key=/etc/azure-ssh/azure-ssh-pub --aksengine-template-url=https://raw.githubusercontent.com/Azure/aks-engine/master/examples/kubernetes.json --aksengine-download-url=https://github.com/Azure/aks-engine/releases/download/v0.53.0/aks-engine-v0.53.0-linux-amd64.tar.gz '--test_args=--ginkgo.focus=\[Conformance\]|\[NodeConformance\] --ginkgo.skip=\[Slow\]|\[Serial\]|\[Flaky\]' --ginkgo-parallel=30 --timeout=420m
2020/08/02 14:22:39 Warning: Couldn't find directory src/sigs.k8s.io/cloud-provider-azure under any of GOPATH /home/prow/go, defaulting to /home/prow/go/src/k8s.io/cloud-provider-azure
2020/08/02 14:22:39 main.go:325: Limiting testing to 7h0m0s
2020/08/02 14:22:39 aksengine_helpers.go:353: Reading credentials file /etc/azure-cred/credentials
2020/08/02 14:22:39 process.go:96: Saved XML output to /logs/artifacts/junit_runner.xml.
2020/08/02 14:22:39 process.go:153: Running: bash -c . hack/lib/version.sh && KUBE_ROOT=. kube::version::get_version_vars && echo "${KUBE_GIT_VERSION-}"
2020/08/02 14:22:39 process.go:155: Step 'bash -c . hack/lib/version.sh && KUBE_ROOT=. kube::version::get_version_vars && echo "${KUBE_GIT_VERSION-}"' finished in 414.971586ms
2020/08/02 14:22:39 main.go:312: Something went wrong: error creating deployer: failed to get azure credentials: error parsing credentials file /etc/azure-cred/credentials (1, 59): parsing error: no value can start with C

BenTheElder · 2020-08-02T16:24:54Z

kubernetes/test-infra#17869 bumped a bunch of dependencies including some azure related stuff, and some later PR caused us to actually upgrade to a new image containing these in kubetest.

https://github.com/kubernetes/test-infra/commits/master/kubetest

andrewsykim · 2020-08-02T17:34:02Z

Ah, thanks for clarifying @BenTheElder!

@andyzhangx @feiskyer can you look at this failure when you get a chance please?

feiskyer · 2020-08-03T00:42:08Z

Thanks @BenTheElder @andrewsykim. The error is error parsing credentials file /etc/azure-cred/credentials (1, 59): parsing error: no value can start with C, do we know anyone changed the credential file at that time?

BenTheElder · 2020-08-03T15:37:29Z

Nobody would have changed the credential without being asked to.

feiskyer · 2020-08-04T02:40:36Z

@BenTheElder no worries, the credential issues have been fixed by test-infra-oncall.

BenTheElder · 2020-08-04T04:58:05Z

Circling back, if anyone's curious this was the issue: kubernetes/test-infra#18618 (comment)

basically the format was invalid but the older go-toml version allowed it.

Fix providerless kubelet startup

93e2868

k8s-ci-robot requested review from aojea and BenTheElder July 31, 2020 16:31

liggitt added this to the v1.19 milestone Jul 31, 2020

k8s-ci-robot added area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 31, 2020

k8s-ci-robot assigned BenTheElder Jul 31, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 31, 2020

liggitt added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Jul 31, 2020

k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jul 31, 2020

SergeyKanzhelev reviewed Jul 31, 2020

View reviewed changes

cmd/kubelet/app/options/globalflags_providers.go Show resolved Hide resolved

k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jul 31, 2020

liggitt force-pushed the providerless-kubelet branch from c57adf5 to 93e2868 Compare July 31, 2020 19:50

liggitt removed the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Jul 31, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 31, 2020

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 1, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 1, 2020

k8s-ci-robot merged commit 8f835d5 into kubernetes:master Aug 2, 2020

liggitt deleted the providerless-kubelet branch August 2, 2020 05:03

andyzhangx mentioned this pull request Aug 3, 2020

fix go-toml issue kubernetes/test-infra#18618

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix providerless kubelet startup #93607

Fix providerless kubelet startup #93607

liggitt commented Jul 31, 2020

BenTheElder commented Jul 31, 2020

liggitt commented Jul 31, 2020

SergeyKanzhelev Jul 31, 2020

liggitt commented Jul 31, 2020

k8s-ci-robot commented Jul 31, 2020

liggitt commented Jul 31, 2020 •

edited

Loading

BenTheElder commented Jul 31, 2020

aojea commented Jul 31, 2020

dims commented Aug 1, 2020

BenTheElder commented Aug 1, 2020

aojea commented Aug 1, 2020

liggitt commented Aug 1, 2020

liggitt commented Aug 1, 2020

andrewsykim commented Aug 2, 2020

BenTheElder commented Aug 2, 2020

BenTheElder commented Aug 2, 2020

BenTheElder commented Aug 2, 2020

andrewsykim commented Aug 2, 2020

feiskyer commented Aug 3, 2020

BenTheElder commented Aug 3, 2020 •

edited

Loading

feiskyer commented Aug 4, 2020

BenTheElder commented Aug 4, 2020

		@@ -84,9 +84,7 @@ func addCredentialProviderFlags(fs *pflag.FlagSet) {
		global := pflag.CommandLine

Fix providerless kubelet startup #93607

Fix providerless kubelet startup #93607

Conversation

liggitt commented Jul 31, 2020

BenTheElder commented Jul 31, 2020

liggitt commented Jul 31, 2020

SergeyKanzhelev Jul 31, 2020

Choose a reason for hiding this comment

liggitt commented Jul 31, 2020

k8s-ci-robot commented Jul 31, 2020

liggitt commented Jul 31, 2020 • edited Loading

BenTheElder commented Jul 31, 2020

aojea commented Jul 31, 2020

dims commented Aug 1, 2020

BenTheElder commented Aug 1, 2020

aojea commented Aug 1, 2020

liggitt commented Aug 1, 2020

liggitt commented Aug 1, 2020

andrewsykim commented Aug 2, 2020

BenTheElder commented Aug 2, 2020

BenTheElder commented Aug 2, 2020

BenTheElder commented Aug 2, 2020

andrewsykim commented Aug 2, 2020

feiskyer commented Aug 3, 2020

BenTheElder commented Aug 3, 2020 • edited Loading

feiskyer commented Aug 4, 2020

BenTheElder commented Aug 4, 2020

liggitt commented Jul 31, 2020 •

edited

Loading

BenTheElder commented Aug 3, 2020 •

edited

Loading