Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added function to create kubeconfig for addon-manager #75675

Merged
merged 1 commit into from May 4, 2019

Conversation

@mwwolters
Copy link
Contributor

commented Mar 25, 2019

/kind cleanup

This is the first step toward migrating the addon manager off of the master insecure port.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

@mwwolters: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

Hi @mwwolters. 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.

@mwwolters

This comment has been minimized.

Copy link
Contributor Author

commented Mar 25, 2019

/assign dekkagaijin
/sig gcp

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

@mwwolters: GitHub didn't allow me to assign the following users: dekkagaijin.

Note that only kubernetes members and repo collaborators can be assigned and that issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign dekkagaijin
/sig gcp

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.

@mwwolters

This comment has been minimized.

Copy link
Contributor Author

commented Mar 25, 2019

@k8s-ci-robot k8s-ci-robot added size/M and removed size/S labels Mar 27, 2019

@mwwolters

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

/assign @jszczepkowski

@mwwolters

This comment has been minimized.

Copy link
Contributor Author

commented Apr 15, 2019

/assign MaciekPytel

@MrHohn

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

/ok-to-test
/assign

@MrHohn
Copy link
Member

left a comment

LGTM overall

@@ -1096,6 +1097,30 @@ current-context: cluster-autoscaler
EOF
}

function create-addonmanager-kubeconfig {

This comment has been minimized.

Copy link
@MrHohn

MrHohn Apr 18, 2019

Member

It feels like these create-*-kubeconfig funcs are mostly the same. Though not sure if it's worth it for refactoring.

This comment has been minimized.

Copy link
@mwwolters

mwwolters Apr 22, 2019

Author Contributor

Added the function and changed in most places a kubeconfig is made.

env:
- name: KUBECTL_EXTRA_PRUNE_WHITELIST
value: {{kubectl_extra_prune_whitelist}}
- name: KUBECTL_OPTS
value: '--kubeconfig=/etc/srv/kubernetes/addon-manager/kubeconfig'

This comment has been minimized.

Copy link
@mwwolters

mwwolters Apr 22, 2019

Author Contributor

I think they do, added and am testing.

@@ -594,6 +594,7 @@ function create-master-auth {
if [[ -n "${NODE_PROBLEM_DETECTOR_TOKEN:-}" ]]; then
append_or_replace_prefixed_line "${known_tokens_csv}" "${NODE_PROBLEM_DETECTOR_TOKEN}," "system:node-problem-detector,uid:node-problem-detector"
fi
append_or_replace_prefixed_line "${known_tokens_csv}" "${ADDON_MANAGER_TOKEN}," "system:addon-manager,uid:addon-manager"

This comment has been minimized.

Copy link
@MrHohn

MrHohn Apr 18, 2019

Member

Probably check if ${ADDON_MANAGER_TOKEN} is set like the others?

This comment has been minimized.

Copy link
@MrHohn

MrHohn Apr 18, 2019

Member

Does this need to match the system:addon-manager-role role we create in create-apply-addon-manager-rbac()?

This comment has been minimized.

Copy link
@mwwolters

mwwolters Apr 22, 2019

Author Contributor
  1. Added the check.
  2. In this case it needs to be the user which is bound to that role.
@@ -2502,6 +2527,13 @@ function start-kube-addons {
local -r src_dir="${KUBE_HOME}/kube-manifests/kubernetes/gci-trusty"
local -r dst_dir="/etc/kubernetes/addons"

# need kube-apiserver to be ready
until kubectl get nodes; do

This comment has been minimized.

Copy link
@MrHohn

MrHohn Apr 18, 2019

Member

We already have two places (e.g. update-legacy-addon-node-labels()) that do this check. Maybe make a wait-til-apiserver-ready() func and put that under main?

This comment has been minimized.

Copy link
@mwwolters

mwwolters Apr 22, 2019

Author Contributor

Added.

This comment has been minimized.

Copy link
@MrHohn

MrHohn Apr 23, 2019

Member

Thanks! Can we run wait-till-apiserver-ready just once so individual step won't need to check explicitly?

    start-kube-scheduler
    wait-till-apiserver-ready
    start-kube-addons
@dekkagaijin

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

/priority important-soon

@MrHohn

This comment has been minimized.

Copy link
Member

commented May 3, 2019

Thanks for making this working :) (I can't approve though.)
/lgtm
/approve

@dekkagaijin

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

/test pull-kubernetes-e2e-gce
/test pull-kubernetes-integration

@dekkagaijin

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

Need approval from cluster/gce/OWNERS
/assign cjcullen

@dekkagaijin

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

/test pull-kubernetes-kubemark-e2e-gce-big

@@ -2765,6 +2700,7 @@ function start-lb-controller {
prepare-log-file /var/log/glbc.log
setup-addon-manifests "addons" "cluster-loadbalancing/glbc"
setup-addon-manifests "addons" "rbac/cluster-loadbalancing/glbc"
create-kubeconfig "l7-lb-controller" ${GCE_GLBC_TOKEN}
create-l7-lb-controller-kubeconfig

This comment has been minimized.

Copy link
@dekkagaijin

dekkagaijin May 3, 2019

Contributor

delete

@mwwolters mwwolters force-pushed the mwwolters:addon-manager-kubeconfig branch from c75a9d7 to f584954 May 3, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label May 3, 2019

@dekkagaijin

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label May 3, 2019

@dekkagaijin

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

/test pull-kubernetes-kubemark-e2e-gce-big

@dekkagaijin

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

🎉 🎉 🎉 🎉

@@ -1987,7 +1920,7 @@ function setup-etcd-encryption {

# Updates node labels used by addons.
function update-legacy-addon-node-labels() {
# need kube-apiserver to be ready
# need kube-api-server to be ready

This comment has been minimized.

Copy link
@cjcullen

cjcullen May 3, 2019

Member

Nit: it's usually just "kube-apiserver"

@@ -597,6 +597,9 @@ function create-master-auth {
if [[ -n "${GCE_GLBC_TOKEN:-}" ]]; then
append_or_replace_prefixed_line "${known_tokens_csv}" "${GCE_GLBC_TOKEN}," "system:controller:glbc,uid:system:controller:glbc"
fi
if [[ -n "${ADDON_MANAGER_TOKEN:-}" ]]; then
append_or_replace_prefixed_line "${known_tokens_csv}" "${ADDON_MANAGER_TOKEN}," "system:addon-manager,admin,system:masters"

This comment has been minimized.

Copy link
@cjcullen

cjcullen May 3, 2019

Member

The entry after the "Name" is the "UID." It's mostly not used, but setting it to "admin" is confusing. Make it uid:system:addon-manager.

@mwwolters mwwolters force-pushed the mwwolters:addon-manager-kubeconfig branch from f584954 to 1456979 May 3, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label May 3, 2019

@dekkagaijin

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label May 3, 2019

@cjcullen

This comment has been minimized.

Copy link
Member

commented May 4, 2019

/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented May 4, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cjcullen, MrHohn, mwwolters

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dekkagaijin

This comment has been minimized.

Copy link
Contributor

commented May 4, 2019

/test pull-kubernetes-e2e-gce

@k8s-ci-robot k8s-ci-robot merged commit 5f8d290 into kubernetes:master May 4, 2019

20 checks passed

cla/linuxfoundation mwwolters authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.