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

Disable dashboard by default #6804

Merged
merged 1 commit into from
Oct 11, 2020
Merged

Disable dashboard by default #6804

merged 1 commit into from
Oct 11, 2020

Conversation

hafe
Copy link
Contributor

@hafe hafe commented Oct 7, 2020

Users should opt in for features and not opt out.

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change

/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

Keep the default deployment as small as possible. Users should opt in for features and not opt out.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

action required: dashboard deployment needs to be explicitly configured

@hafe
Disable dashboard by default
788a13e 
Users should opt in for features and not opt out.
@k8s-ci-robot k8s-ci-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 7, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @hafe. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

@k8s-ci-robot k8s-ci-robot requested review from bozzo and holmsten October 7, 2020 20:01
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 7, 2020
@oomichi
Copy link
Contributor

oomichi commented Oct 8, 2020

I cannot find document which says Kubespray deployment should be minimum as possible in this repository.
And this could be an impact for users if not having concrete reason officially, I feel.

I could understand disabling the dashboard in the default because I expected most users just use kubectl command and not use the dashboard. However my colleagues want to use the dashboard to understand the situation easily and the dashboard is deployed at the default with Kubespray in our clusters.
I'd like to see other opinions.

/cc @oomichi

@k8s-ci-robot k8s-ci-robot requested a review from oomichi October 8, 2020 00:36
@hafe
Copy link
Contributor Author

hafe commented Oct 8, 2020

I cannot find document which says Kubespray deployment should be minimum as possible in this repository.
And this could be an impact for users if not having concrete reason officially, I feel.

I could understand disabling the dashboard in the default because I expected most users just use kubectl command and not use the dashboard. However my colleagues want to use the dashboard to understand the situation easily and the dashboard is deployed at the default with Kubespray in our clusters.
I'd like to see other opinions.

/cc @oomichi

Reducing the scope of Kubespray has been discussed in slack and in the 3.0 issue.

Disabling the dashboard by default was brought as something wanted.

If you want it is still there and you can just flip the value of a variable. Of preferable (as discussed), use the official upstream supported way of deploying

@floryut
Copy link
Member

floryut commented Oct 9, 2020 •
edited
Loading

As long as we put a big (real big) notice in the release note, I think we are safe to go.
And I kind of agree that a lot of users don't want the dashboard bundle with their kubernetes install.

So I would go with @hafe and roll this PR but as you pointed @oomichi maybe we need to discuss this with @Miouge1 and @EppO (and others if anyone as something to say about that)

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 9, 2020
@woopstar
Copy link
Member

woopstar commented Oct 9, 2020

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hafe, woopstar

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2020
@oomichi
Copy link
Contributor

oomichi commented Oct 9, 2020

Happy to see opinions, thanks.
It would be nice to go with this PR by having the release note.

@LuckySB
Copy link
Contributor

LuckySB commented Oct 11, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 11, 2020
@k8s-ci-robot k8s-ci-robot merged commit 92b1166 into kubernetes-sigs:master Oct 11, 2020
erulabs added a commit to kubesail/kubespray that referenced this pull request Oct 13, 2020
@erulabs
Merge branch 'master' of https://github.com/kubernetes-sigs/kubespray
49b541f 
* 'master' of https://github.com/kubernetes-sigs/kubespray:
  change owner to root for bin_dir directory (kubernetes-sigs#6814)
  Modify imagepullpolicy (kubernetes-sigs#6816)
  fix: add tags for set facts nodelocaldns (kubernetes-sigs#6813)
  Make reset work for crio (kubernetes-sigs#6812)
  Added option to force apiserver and respective client certificate to … (kubernetes-sigs#6403)
  cleanup kubelet_deployment_type (kubernetes-sigs#6815)
  allow pre-existing floating IPs to be specified with k8s_master_fips (kubernetes-sigs#6755)
  Fix line-spacing in no_proxy.yml (kubernetes-sigs#6810)
  Fix handler naming issue for Kubeadm | kubelet (kubernetes-sigs#6803)
  Disable dashboard by default (kubernetes-sigs#6804)
  Chmod kubeconfig to avoid group-readable (kubernetes-sigs#6800)
  Update bunch of dependencies (kubernetes-sigs#6801)
  If no_proxy_exclude_workers is true, workers will be excluded from the no_proxy variable.  This prevents docker engine restarting when scaling workers. (kubernetes-sigs#6520)
@floryut floryut mentioned this pull request Dec 19, 2020
Closed
LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Jan 16, 2021
@hafe @LuckySB
Disable dashboard by default (kubernetes-sigs#6804)
8beffec 
Users should opt in for features and not opt out.
@oomichi oomichi mentioned this pull request Feb 23, 2021
Merged
oomichi pushed a commit to oomichi/kubespray that referenced this pull request Mar 1, 2021
Update dashboard_enabled on sample
5b65fae 
Since kubernetes-sigs#6804
dashboard_enabled has been false by default.
However we forgot to update it on sample inventory and it made
confusion.
This updates the sample inventory.
k8s-ci-robot pushed a commit that referenced this pull request Mar 2, 2021
@oomichi
Update dashboard_enabled on sample (#7316)
ef351e0 
Since #6804
dashboard_enabled has been false by default.
However we forgot to update it on sample inventory and it made
confusion.
This updates the sample inventory.
LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Apr 6, 2021
@oomichi @LuckySB
Update dashboard_enabled on sample (kubernetes-sigs#7316)
70649db 
Since kubernetes-sigs#6804
dashboard_enabled has been false by default.
However we forgot to update it on sample inventory and it made
confusion.
This updates the sample inventory.
msimonin pushed a commit to msimonin/enos-kubernetes that referenced this pull request May 5, 2022
@jonglezb
Re-enable dashboard
a9934e8 
It was disabled by default in Kubespray recently: kubernetes-sigs/kubespray#6804
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bozzo bozzo Awaiting requested review from bozzo

@holmsten holmsten Awaiting requested review from holmsten

@oomichi oomichi Awaiting requested review from oomichi

Assignees

@LuckySB LuckySB

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@hafe @k8s-ci-robot @oomichi @floryut @woopstar @LuckySB