Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

Making sure service-account-private-key-file arg is available in kube… #25

Merged
merged 1 commit into from
Jun 18, 2018

Conversation

ktsakalozos
Copy link

…-controller-manager

@hyperbolic2346
Copy link

LGTM. Thanks.

@hyperbolic2346 hyperbolic2346 merged commit dd9238e into rye/snaps Jun 18, 2018
# By mistake --service-account-private-key-file was marked as deprecated
# https://github.com/kubernetes/kubernetes/pull/62722
# Should be fixed soon by https://github.com/kubernetes/kubernetes/pull/60270/files#diff-fc9db90cc6b68d7c3ca838a8512447e9R61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't how I read those PRs. It looks to me like the flag was marked deprecated as far back as 1.8, with this PR: kubernetes/kubernetes#50289

1.8 release note:

The kube-cloud-controller-manager flag --service-account-private-key-file was non-functional and is now deprecated.

60270 was already merged, but I think it's just a big refactor that moved stuff around. Still looking into it.

62722 has not been merged yet, but it will completely remove the --service-account-private-key-file flag.

Is the correct fix here to get the charm to stop setting it?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, nevermind. I see now. Those are cloud-controller-manager not kube-controller-manager.

/me looks again

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I can't make sense of what they're doing. Looks like the deprecation on kube-controller-manager was by accident, though, when they merged the backend options code for kube-controller-manager and kube-cloud-controller-manager.

LGTM, sorry for the false alarm 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants