Add missing cli options for kube-controller-manager and kube-scheduler

[Evalle]

PR Summary:

• Adds missing cli options for kube-controller-manager and kube-scheduler
• Fixes Missing cli options for kube-controller-manager and kube-scheduler #9695

[k8s-ci-robot]

Welcome @Evalle!

It looks like this is your first PR to kubernetes/kops 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kops has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @Evalle. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

[Evalle]

/assign @justinsb

[hakman]

/ok-to-test

[hakman]

@Evalle Thanks for taking this. Please run make apimachinery and probably also make crds and commit the result. These should fix the tests.

[Evalle]

Hi @hakman thanks for the review!
I actually run make apimachinery crds prior to commit

[hakman]

Ah... right. I think there is something broken there. Please run hack/make-apimachinery.sh instead.

[Evalle]

It didn't make any changes

❯ pwd
/Projects/kops
❯ ./hack/make-apimachinery.sh
go: downloading k8s.io/code-generator v0.18.6
go: downloading github.com/spf13/pflag v1.0.5
go: downloading k8s.io/gengo v0.0.0-20200710205751-c0d492a0f3ca
go: downloading k8s.io/klog v1.0.0
go: downloading k8s.io/klog/v2 v2.0.0
go: downloading golang.org/x/tools v0.0.0-20200626171337-aa94e735be7f
go: downloading github.com/go-logr/logr v0.1.0
go: downloading golang.org/x/mod v0.3.0
go: downloading golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543
golang.org/x/mod/semver
golang.org/x/xerrors/internal
github.com/go-logr/logr
golang.org/x/xerrors
golang.org/x/tools/go/ast/astutil
golang.org/x/tools/internal/event/label
golang.org/x/tools/internal/event/keys
github.com/spf13/pflag
golang.org/x/mod/module
golang.org/x/tools/internal/event/core
golang.org/x/tools/internal/fastwalk
golang.org/x/tools/internal/event
golang.org/x/tools/internal/gopathwalk
golang.org/x/tools/internal/gocommand
k8s.io/gengo/types
k8s.io/gengo/namer
k8s.io/klog/v2
k8s.io/klog
k8s.io/gengo/parser
k8s.io/code-generator/pkg/util
golang.org/x/tools/internal/imports
golang.org/x/tools/imports
k8s.io/gengo/generator
k8s.io/gengo/args
k8s.io/code-generator/cmd/conversion-gen/args
k8s.io/code-generator/cmd/conversion-gen/generators
k8s.io/code-generator/cmd/conversion-gen
❯ echo $?
0
❯ git status
On branch ISSUE-9695
Your branch is up to date with 'origin/ISSUE-9695'.

nothing to commit, working tree clean

JFYI I've forked from the master branch, could it be the issue?

[hakman]

May be related to Go version (using 1.15rc2 at the moment) or GOPATH. You should get something like this from make apimachinery:

--- a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go
+++ b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go
@@ -4003,6 +4003,8 @@ func autoConvert_v1alpha2_KubeControllerManagerConfig_To_kops_KubeControllerMana
        out.ConcurrentResourceQuotaSyncs = in.ConcurrentResourceQuotaSyncs
        out.ConcurrentServiceaccountTokenSyncs = in.ConcurrentServiceaccountTokenSyncs
        out.ConcurrentRcSyncs = in.ConcurrentRcSyncs
+       out.AuthenticationKubeconfig = in.AuthenticationKubeconfig
+       out.AuthorizationKubeconfig = in.AuthorizationKubeconfig
        out.EnableProfiling = in.EnableProfiling
        return nil
 }
@@ -4063,6 +4065,8 @@ func autoConvert_kops_KubeControllerManagerConfig_To_v1alpha2_KubeControllerMana
        out.ConcurrentResourceQuotaSyncs = in.ConcurrentResourceQuotaSyncs
        out.ConcurrentServiceaccountTokenSyncs = in.ConcurrentServiceaccountTokenSyncs
        out.ConcurrentRcSyncs = in.ConcurrentRcSyncs
+       out.AuthenticationKubeconfig = in.AuthenticationKubeconfig
+       out.AuthorizationKubeconfig = in.AuthorizationKubeconfig
        out.EnableProfiling = in.EnableProfiling
        return nil
 }
@@ -4212,6 +4216,8 @@ func autoConvert_v1alpha2_KubeSchedulerConfig_To_kops_KubeSchedulerConfig(in *Ku
        out.MaxPersistentVolumes = in.MaxPersistentVolumes
        out.Qps = in.Qps
        out.Burst = in.Burst
+       out.AuthenticationKubeconfig = in.AuthenticationKubeconfig
+       out.AuthorizationKubeconfig = in.AuthorizationKubeconfig
        out.EnableProfiling = in.EnableProfiling
        return nil
 }
@@ -4239,6 +4245,8 @@ func autoConvert_kops_KubeSchedulerConfig_To_v1alpha2_KubeSchedulerConfig(in *ko
        out.MaxPersistentVolumes = in.MaxPersistentVolumes
        out.Qps = in.Qps
        out.Burst = in.Burst
+       out.AuthenticationKubeconfig = in.AuthenticationKubeconfig
+       out.AuthorizationKubeconfig = in.AuthorizationKubeconfig
        out.EnableProfiling = in.EnableProfiling
        return nil
 }

[Evalle]

interesting, I have 1.14.5

❯ go version
go version go1.14.5 darwin/amd64

Let me try with 1.14.6 or 1.15.x

[Evalle]

I think I've found the issue :) it was related to GOPATH indeed, I had kops in both /go/src and in Projects directory.

[hakman]

@Evalle would authorization-always-allow-paths also be needed, or is /healthzenough?

--authorization-always-allow-paths stringSlice     Default: [/healthz]
  A list of HTTP paths to skip during authorization, i.e. these are authorized without contacting the 'core' kubernetes server.

[Evalle]

Hm, I would ask @rifelpet about this

[hakman]

And also @a8j8i8t8: #9726 (comment).

[rifelpet]

Yea it probably makes sense to add --authorization-always-allow-paths since many users of the original two fields might find themselves wanting to override this field too. While you're at it I left a minor grammatical suggestion in the field comments. Once always-allow-paths is added this lgtm. Thanks for picking this up!

[hakman]

@Evalle Please add also the --authorization-always-allow-paths if that's ok with you.

[Evalle]

@hakman yes, I'm working on it ATM :)

[hakman]

Thanks :)

[Evalle]

Should I squash my commits ?

[hakman]

Looks pretty good to me. Congrats on your first Kops PR :)
/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Evalle, hakman

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:

• OWNERS [hakman]

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

[Evalle]

Happy to help :) thanks for the review @hakman

[hakman]

My pleasure! 👍