Disable Kubelet read-only port 10255

[dghubble]

pod-checkpointer creates an insecureClient and secureClient to the kubelet, but only makes use of the former (via 10255). Adapt localParentPods to try to use the secure client first, then the insecure client to support clusters that disable the kubelet read-only port.

Open questions:

• What about we just always use the kubelet secure API (instead of having a fallback)?
• Testing with the secure client, we'll need a ClusterRole with node/proxy get. Surprising since the pod-checkpointer also mounts the admin kubeconfig from the host (possibly not being used).

Background

Today in bootkube, the kubelet read-only port (10255) is enabled and pod-checkpointer uses its /pods endpoint to get all pods (later filters to find parent pods). That all works fine.

We've come a long way toward eliminating the read-only port. Cloud load balancers can now health check the apiserver, Prometheus can use the kubelet secure API to scrape metrics, and heapster can get metrics from the kubelet secure API too.

Running clusters with kubelet read-only-port=0 (disabled), the active pod-checkpointer will log:

failed to list local parent pods, assuming none are running: Get http://127.0.0.1:10255/pods/: dial tcp 127.0.0.1:10255: connect: connection refused

Interestingly, even with localParentPods calls failing on these clusters, recovery from cluster power cycling is unaffected. But I figure if we can eliminate this one last use of the read-only API, all the better.

[coreosbot]

Can one of the admins verify this patch?

[coreosbot]

Can one of the admins verify this patch?

[coreosbot]

Can one of the admins verify this patch?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: dghubble

If they are not already assigned, you can assign the PR to them by writing /assign @dghubble in a comment when ready.

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

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

[dghubble]

I expect I'll need to split the checkpointer change and user-data change for tests to pass.

But manually testing on clusters with --read-only-port=0 and a test checkpointer image from this PR, I was able to resolve the error posted above after adding a ClusterRole:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: pod-checkpointer
rules:
  - apiGroups: [""]
    resources:
      - nodes
      - nodes/proxy
    verbs:
      - get

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  name: pod-checkpointer
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: pod-checkpointer
subjects:
- kind: ServiceAccount
  name: pod-checkpointer
  namespace: kube-system

[rphillips]

Thanks Dalton for looking into this. We just had a conversation about this exact issue. I am worried about the certificate rotation needed for the checkpointer -> kubelet secure connection. There is an edge case where if a node is down for a period of time, comes back, the cert might be expired. Not to mention needing to maintain the cert rotation on that connection.

We are definitely on board to try and remove the insecure port requirement.

[dghubble]

Ah right. bootkube supports using kubelet TLS bootstrap so the concern is if there's no apiserver (e.g. the cluster is powered off) when the certificate expires, then on startup, kubelet starts pod-checkpointer which may not make it far enough* to start the bootstrap-apiserver, to issue the new cert so the kubelet can register. So given the choice, we prefer to at least fallback to 10255.

I can post a standalone PR for the checkpointer to try 10250, then fallback to 10255.

*I'm not sure how that can happen, since clusters that disable read-only today, pod-checkpointer can't contact anything. Yet control plane recovery succeeds.

[dghubble]

Added the ClusterRole and ClusterRoleBinding to give checkpointer permission to perform requests previously done via the kubelet read-only API. Requires #1027 and a checkpointer release before this can pass tests.

[rphillips]

@dghubble can you update the checkpointer hash to: 83e25e5968391b9eb342042c435d1b3eeddb2be1

[dghubble]

Updated, and opened #1031 with just the image change, since we might consider it separate from the feature change of disabling read-only in test / example clusters.

Release Note:

* Disable Kubelet read-only port 10255 (may affect applications)
  * Configure any added apps using the insecure API to use the Kubelet secure API (e.g. heapster, prometheus)
  * If using Kubelet certificate rotation, you may wish to leave read-only port on to mitigate any [chance of interruption](https://github.com/kubernetes-incubator/bootkube/pull/1025#issuecomment-437038376).

[rphillips]

/ok-to-test

[rphillips]

19:17:03 Error: loading manifests: parse file /home/core/assets/manifests/pod-checkpointer-cluster-role.yaml: invalid manifest: yaml: line 7: found unexpected end of stream

[dghubble]

Gah, verbs: ["get'] -> verbs: ["get"]

[rphillips]

coreosbot run e2e calico

[rphillips]

We should leave this network requirement in but label it 'optional'. otherwise this lgtm and tests are passing.

[dghubble]

Updated

[dghubble]

Did we want to add just bump the checkpointer via #1031, then rebase this to follow it? Since the two are somewhat independent. 🤷‍♂️ either way works for me

[rphillips]

@dghubble Thanks! We can rebase this PR now.

[dghubble]

Updated

[rphillips]

coreosbot run e2e calico

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.