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

kube-apiserver: default --enable-logs-handler flag to false #110738

Closed
wants to merge 1 commit into from
Closed

kube-apiserver: default --enable-logs-handler flag to false #110738

wants to merge 1 commit into from

Conversation

pandaamanda
Copy link
Contributor

@pandaamanda pandaamanda commented Jun 23, 2022 •
edited by liggitt
Loading

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Which issue(s) this PR fixes:

ref #77611

Special notes for your reviewer:

Does this PR introduce a user-facing change?

ACTION REQUIRED: kube-apiserver: `--enable-logs-handler` now defaults to false, which disables the `/logs` endpoint served by kube-apiserver. If serving log files within the `/var/log` filepath at the `/logs` endpoint is still desired, the functionality is now opt-in and must be enabled by explicitly setting `--enable-logs-handler=true`

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/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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 Jun 23, 2022
@k8s-ci-robot k8s-ci-robot requested review from aojea and dcbw June 23, 2022 08:17
@pandaamanda pandaamanda changed the title refactor: remove kube-apiserver --enable-logs-handler flag and log-se… refactor: remove kube-apiserver --enable-logs-handler flag and log-serving functionality Jun 23, 2022
@k8s-ci-robot k8s-ci-robot added area/apiserver area/provider/gcp Issues or PRs related to gcp provider area/test 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/network Categorizes an issue or PR as relevant to SIG Network. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 23, 2022
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/conformance Issues or PRs related to kubernetes conformance tests sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/release Categorizes an issue or PR as relevant to SIG Release. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 23, 2022
@leilajal
Copy link
Contributor

/cc @Jefftree @apelisse
/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 Jun 23, 2022
@Jefftree
Copy link
Member

Other than the /logs change, are the other OpenAPI changes related to this PR?

@liggitt
Copy link
Member

liggitt commented Jul 8, 2022

This lgtm. I would like an ack from an api-machinery lead on the default change.

/assign @deads2k @lavalamp

cc @kubernetes/sig-security-pr-reviews for visibility to the change to default to not exposing API server machine logs

cc @kubernetes/sig-cluster-lifecycle @neolit123 for visibility to kubeadm... would you expect kubeadm to go with the new default-off value?

@neolit123
Copy link
Member

@neolit123 for visibility to kubeadm... would you expect kubeadm to go with the new default-off value?

SGTM, kubeadm uses the apiserver default for this flag. opting-in users may have to adjust after seeing the release note.

@pandaamanda
Copy link
Contributor Author

/assign @deads2k @lavalamp PTAL, tks.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 17, 2022
@k8s-ci-robot
Copy link
Contributor

@pandaamanda: PR needs rebase.

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 k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 10, 2022
@lavalamp
Copy link
Member

Sorry I didn't see this until now. I think users need a release of warning before the default changes. It is reasonable to have interpreted the deprecation notice as meaning that the feature would be on all the time when the flag was removed. Additionally I'm not sure what to do about the openapi -- I kind of think we should publish it even if the flag defaults off.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 10, 2022
@dims
Copy link
Member

dims commented Dec 12, 2022

If you still need this PR then please rebase, if not, please close the PR

@sftim
Copy link
Contributor

sftim commented Jan 10, 2023

If the PR for kubernetes/website#38870 lands first, we should update those docs when this goes in.

@sftim
Copy link
Contributor

sftim commented Jan 10, 2023

Relevant to #81125

This was referenced Jan 10, 2023
Open
Merged
@liggitt
Copy link
Member

liggitt commented Jan 20, 2023

FWIW, there's a KEP in progress to expand access to host logs on the kubelet side, which doesn't seem coherent with the goal of reducing access to host logs via the apiserver... commented at #96120 (review) and cross-linked here.

@jayunit100
Copy link
Member

this PR doesnt have an open issue assocaited with it. Can we fix that ? Otherwise, theres no way to evaluate wether theres decision precedence that will blog the node logs were trying to get in #96120

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/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.

@liggitt liggitt reopened this Feb 21, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pandaamanda
Once this PR has been reviewed and has the lgtm label, please ask for approval from lavalamp. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@k8s-ci-robot
Copy link
Contributor

@pandaamanda: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubernetes-node-e2e-containerd f0b4dd0 link true /test pull-kubernetes-node-e2e-containerd
pull-kubernetes-e2e-gce f0b4dd0 link true /test pull-kubernetes-e2e-gce
pull-kubernetes-conformance-kind-ipv6-parallel f0b4dd0 link false /test pull-kubernetes-conformance-kind-ipv6-parallel
pull-kubernetes-e2e-kind f0b4dd0 link true /test pull-kubernetes-e2e-kind
pull-kubernetes-integration f0b4dd0 link true /test pull-kubernetes-integration
pull-kubernetes-e2e-gce-100-performance f0b4dd0 link true /test pull-kubernetes-e2e-gce-100-performance
pull-kubernetes-dependencies f0b4dd0 link true /test pull-kubernetes-dependencies
pull-kubernetes-unit f0b4dd0 link true /test pull-kubernetes-unit
pull-kubernetes-e2e-kind-ipv6 f0b4dd0 link true /test pull-kubernetes-e2e-kind-ipv6
pull-kubernetes-typecheck f0b4dd0 link true /test pull-kubernetes-typecheck
pull-kubernetes-conformance-kind-ga-only-parallel f0b4dd0 link true /test pull-kubernetes-conformance-kind-ga-only-parallel
pull-kubernetes-e2e-gci-gce-ipvs f0b4dd0 link false /test pull-kubernetes-e2e-gci-gce-ipvs
pull-kubernetes-e2e-ubuntu-gce-network-policies f0b4dd0 link false /test pull-kubernetes-e2e-ubuntu-gce-network-policies
pull-kubernetes-e2e-gci-gce-ingress f0b4dd0 link false /test pull-kubernetes-e2e-gci-gce-ingress
pull-kubernetes-verify f0b4dd0 link true /test pull-kubernetes-verify
pull-kubernetes-verify-govet-levee f0b4dd0 link true /test pull-kubernetes-verify-govet-levee

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/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.

@dims dims mentioned this pull request Jun 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aojea aojea Awaiting requested review from aojea

@dcbw dcbw Awaiting requested review from dcbw

@apelisse apelisse Awaiting requested review from apelisse

@Jefftree Jefftree Awaiting requested review from Jefftree

@liggitt liggitt Awaiting requested review from liggitt

Assignees

@lavalamp lavalamp

@Jefftree Jefftree

@deads2k deads2k

Labels
area/apiserver area/conformance Issues or PRs related to kubernetes conformance tests area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework 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/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/security Categorizes an issue or PR as relevant to SIG Security. 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
No milestone
Development

Successfully merging this pull request may close these issues.