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.

Sign up for GitHub

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

Jump to bottom

[KEP-2395] Phase 4 - Disabling In-Tree Providers #117503

Merged
merged 3 commits into from
Sep 2, 2023
Merged

[KEP-2395] Phase 4 - Disabling In-Tree Providers #117503

merged 3 commits into from
Sep 2, 2023

Conversation

dims
Copy link
Member

@dims dims commented Apr 20, 2023 •
edited

KEP-2395 states that Beta was targeted for v1.26, So we missed the boat and now it's v1.28 v1.29 cycle :) Let's see if we can land this now and adjust the CI jobs as needed.

https://github.com/kubernetes/enhancements/tree/master/keps/sig-cloud-provider/2395-removing-in-tree-cloud-providers#phase-4---disabling-in-tree-providers

DisableCloudProviders - this feature gate will disable any functionality in kube-apiserver, kube-controller-manager and kubelet related to the --cloud-provider component flag.

DisableKubeletCloudCredentialProvider - this feature gate will disable in-tree functionality in the kubelet to authenticate to the Azure and GCP container registries for image pull credentials.

KEP metadata update in https://github.com/kubernetes/enhancements/pull/4171/files

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

In tree cloud providers are now switched off by default. Please use DisableCloudProviders and DisableKubeletCloudCredentialProvider feature flags if you still need this functionality.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Apr 20, 2023
@dims
Copy link
Member Author

dims commented Apr 20, 2023

/assign @andrewsykim @nckturner

@dims
Copy link
Member Author

dims commented Apr 20, 2023

/sig cloud-provider

@k8s-ci-robot k8s-ci-robot added sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Apr 20, 2023
@dims
Copy link
Member Author

dims commented Apr 20, 2023

/priority important-soon

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Apr 20, 2023
@k8s-ci-robot k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Apr 20, 2023
@dims dims mentioned this pull request Apr 20, 2023
Merged
@BenTheElder
Copy link
Member

There's "unit tests" for these scripts that will need tweaking

So this generally means CI will not be covering them now?

@dims dims force-pushed the phase-4-kep-2395-removing-in-tree-cloud-providers branch from e1ae4f5 to 68c59fc Compare April 20, 2023 17:13
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 20, 2023
@dims
Copy link
Member Author

dims commented Apr 20, 2023 •
edited

@BenTheElder that's what i am trying to figure out here, which tests we will fail and will need another place/way to test. so far it seems to be just TestMetadataClient

@dims dims force-pushed the phase-4-kep-2395-removing-in-tree-cloud-providers branch from 68c59fc to a7901cd Compare April 20, 2023 18:22
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 20, 2023
@dims dims force-pushed the phase-4-kep-2395-removing-in-tree-cloud-providers branch from a7901cd to aa9a7ec Compare April 20, 2023 18:23
@SergeyKanzhelev
Copy link
Member

/cc @ruiwen-zhao

@dims dims force-pushed the phase-4-kep-2395-removing-in-tree-cloud-providers branch from cebdf42 to ceaed50 Compare September 2, 2023 17:08
@dims
Copy link
Member Author

dims commented Sep 2, 2023

Update the comment for DisableKubeletCloudCredentialProviders at https://github.com/kubernetes/kubernetes/blob/4619f7e9d9d64a0ae0b17270496ca3f7c67d24e8/pkg/features/kube_features.go#L240C2-L240C40 as well?

Done!

tzneal
tzneal approved these changes Sep 2, 2023
View reviewed changes
Copy link
Contributor

@tzneal tzneal left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 2, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 64f2e107fa5e87e05b8de031e282e4fa89135e28

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, tzneal

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

@dims dims mentioned this pull request Sep 2, 2023
Closed
@k8s-ci-robot k8s-ci-robot merged commit a607dfb into kubernetes:master Sep 2, 2023
14 of 15 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.29 milestone Sep 2, 2023
@aojea
Copy link
Member

aojea commented Sep 2, 2023

Thanks dims

@aojea
Copy link
Member

aojea commented Sep 2, 2023

cc: @andrewsykim @liggitt

This was referenced Sep 3, 2023
Closed
Closed
@aojea
Copy link
Member

aojea commented Sep 3, 2023

@aojea please look at failures in the worker node kubelet in the latest pull-kubernetes-e2e-gci-gce-ingress CI job:

from log - https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/117503/pull-kubernetes-e2e-gci-gce-ingress/1697765618037559296/artifacts/bootstrap-e2e-minion-group-15cv/kubelet.log:

Sep 02 00:46:52.046603 bootstrap-e2e-minion-group-15cv kubelet[10550]: E0902 00:46:52.046567   10550 run.go:74] "command failed" err="failed to run Kubelet: cannot create certificate signing request: certificatesigningrequests.certificates.k8s.io is forbidden: User \"kubelet\" cannot create resource \"certificatesigningrequests\" in API group \"certificates.k8s.io\" at the cluster scope"

there are more failures in other components

role=extension-apiserver-authentication-reader --serviceaccount=YOUR_NS:YOUR_SA'
unable to load configmap based request-header-client-ca-file: configmaps "extension-apiserver-authentication" is forbidden: User "system:cloud-controller-manager" cannot get resource "configmaps" in API group "" in the namespace "kube-system"
Error: unable to load configmap based request-header-client-ca-file: configmaps "extension-apiserver-authentication" is forbidden: User "system:cloud-controller-manager" cannot get resource "configmaps" in API group "" in the namespace "kube-system"
Usage:

https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/117503/pull-kubernetes-e2e-gci-gce-ingress/1697765618037559296/artifacts/bootstrap-e2e-master/cloud-controller-manager.log

it seems to be a more general rbac problem

@aojea
Copy link
Member

aojea commented Sep 3, 2023

the kube-addong-manager is not able to register

https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/117503/pull-kubernetes-e2e-gci-gce-ingress/1697765618037559296/artifacts/bootstrap-e2e-master/kube-addon-manager.log

Error from server (NotFound): serviceaccounts "default" not found
WRN: == Error getting default service account, retry in 0.5 second ==

@aojea
Copy link
Member

aojea commented Sep 3, 2023

the kube-master-configuration scripts fails

master","boot_id":"d7b51975c2f94a478a4346b249c1a8dd","timestamp":"2023-09-02T00:45:40.002119670Z","bootstrap_status":{"step_name":"UpdateLegacyAddonNodeLabels","status":"COMPLETED","status_reason":"","latency":"1.23726s"}}
Sep 02 00:45:48.570082 bootstrap-e2e-master configure-helper.sh[12179]: error: the server doesn't have a resource type "volumesnapshotclasses"
Sep 02 00:45:58.668565 bootstrap-e2e-master configure-helper.sh[12327]: error: the server doesn't have a resource type "volumesnapshotclasses"
Sep 02 00:46:08.861606 bootstrap-e2e-master configure-helper.sh[12482]: error: the server doesn't have a resource type "volumesnapshotclasses"
Sep 02 00:46:19.034558 bootstrap-e2e-master configure-helper.sh[12649]: error: the server doesn't have a resource type "volumesnapshotclasses"
Sep 02 00:46:29.131068 bootstrap-e2e-master configure-helper.sh[12797]: error: the server doesn't have a resource type "volumesnapshotclasses"
Sep 02 00:46:39.230542 bootstrap-e2e-master configure-helper.sh[13041]: error: the server doesn't have a resource type "volumesnapshotclasses"

https://storage.googleapis.com/kubernetes-jenkins/pr-logs/pull/117503/pull-kubernetes-e2e-gci-gce-ingress/1697765618037559296/artifacts/bootstrap-e2e-master/kube-master-configuration.log

aojea
aojea reviewed Sep 3, 2023
View reviewed changes
cluster/gce/config-default.sh
export FEATURE_GATES="${FEATURE_GATES},DisableKubeletCloudCredentialProviders=True,DisableCloudProviders=True"
fi
fi
export ENABLE_AUTH_PROVIDER_GCP="${ENABLE_AUTH_PROVIDER_GCP:-false}"
Copy link
Member

Choose a reason for hiding this comment

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

if [[ "${CLOUD_PROVIDER_FLAG:-}" == "external" ]]; then
  export ENABLE_AUTH_PROVIDER_GCP=true

@alexzielenski
Copy link
Contributor

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 5, 2023
@sftim
Copy link
Contributor

sftim commented Sep 6, 2023

Changelog suggestion

-In tree cloud providers are now switched off by default. Please use DisableCloudProviders and DisableKubeletCloudCredentialProvider feature flags if you still need this functionality.
+In-tree cloud provider integrations are now switched off by default. Please use the `DisableCloudProviders` and `DisableKubeletCloudCredentialProvider` feature flags if you still need this functionality.

@sftim
Copy link
Contributor

sftim commented Sep 6, 2023

Relevant to kubernetes/enhancements#2395

@sftim sftim mentioned this pull request Sep 6, 2023
Open
8 tasks
@pacoxu pacoxu mentioned this pull request Sep 12, 2023
Closed
@wojtek-t wojtek-t mentioned this pull request Oct 2, 2023
Merged
@pacoxu pacoxu mentioned this pull request Nov 23, 2023
Closed
This was referenced Jan 8, 2024
Merged
Open
@embik embik mentioned this pull request Feb 5, 2024
Merged
@Vyom-Yadav Vyom-Yadav mentioned this pull request Mar 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elmiko elmiko elmiko left review comments

@aojea aojea aojea left review comments

@andrewsykim andrewsykim andrewsykim left review comments

@tzneal tzneal tzneal approved these changes

@liggitt liggitt Awaiting requested review from liggitt

@thockin thockin Awaiting requested review from thockin

@ruiwen-zhao ruiwen-zhao Awaiting requested review from ruiwen-zhao

Assignees

@tzneal tzneal

@nckturner nckturner

@aojea aojea

@andrewsykim andrewsykim

@upodroid upodroid

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/provider/gcp Issues or PRs related to gcp provider area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.29
Development

Successfully merging this pull request may close these issues.

None yet