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

Helm CRDS are installed despite turning off helm-controller #8701

Closed
harsimranmaan opened this issue Oct 19, 2023 · 5 comments · Fixed by #8702
Closed

Helm CRDS are installed despite turning off helm-controller #8701

harsimranmaan opened this issue Oct 19, 2023 · 5 comments · Fixed by #8702
Assignees
Milestone

Comments

@harsimranmaan
Copy link
Contributor

What's the source for the helm CRDs, they are getting installed even when the helm controller is not enabled

 journalctl -u k3s | grep -i helm
Oct 19 18:51:02 ip-XXXX.us-west-2.compute.internal k3s[6713]: time="2023-10-19T18:51:02Z" level=info msg="Applying CRD helmcharts.helm.cattle.io"
Oct 19 18:51:02 ip-XXXX.us-west-2.compute.internal k3s[6713]: time="2023-10-19T18:51:02Z" level=info msg="Applying CRD helmchartconfigs.helm.cattle.io"
Oct 19 18:51:02 ip-XXXX.us-west-2.compute.internal k3s[6713]: I1019 18:51:02.106789    6713 handler.go:232] Adding GroupVersion helm.cattle.io v1 to ResourceManager
Oct 19 18:51:02 ip-XXXX.us-west-2.compute.internal k3s[6713]: I1019 18:51:02.107290    6713 handler.go:232] Adding GroupVersion helm.cattle.io v1 to ResourceManager
Oct 19 18:51:02 ip-XXXX.us-west-2.compute.internal k3s[6713]: I1019 18:51:02.109724    6713 handler.go:232] Adding GroupVersion helm.cattle.io v1 to ResourceManager
Oct 19 18:51:02 ip-XXXX.us-west-2.compute.internal k3s[6713]: I1019 18:51:02.112423    6713 handler.go:232] Adding GroupVersion helm.cattle.io v1 to ResourceManager
Oct 19 18:51:13 ip-XXXX.us-west-2.compute.internal k3s[6713]: time="2023-10-19T18:51:13Z" level=info msg="Imported docker.io/rancher/klipper-helm:v0.8.2-build20230815"
Oct 19 18:51:15 ip-XXXX.us-west-2.compute.internal k3s[6713]: I1019 18:51:15.329574    6713 resource_quota_monitor.go:210] "QuotaMonitor created object count evaluator" resource="helmchartconfigs.helm.cattle.io"
Oct 19 18:51:15 ip-XXXX.us-west-2.compute.internal k3s[6713]: I1019 18:51:15.329681    6713 resource_quota_monitor.go:210] "QuotaMonitor created object count evaluator" resource="helmcharts.helm.cattle.io"
Oct 19 18:56:00 ip-XXXX.us-west-2.compute.internal k3s[6713]: I1019 18:56:00.098836    6713 handler.go:232] Adding GroupVersion helm.cattle.io v1 to ResourceManager
Oct 19 18:56:00 ip-XXXX.us-west-2.compute.internal k3s[6713]: I1019 18:56:00.098869    6713 handler.go:232] Adding GroupVersion helm.cattle.io v1 to ResourceManager

Environmental Info:
K3s Version:

k3s -v
k3s version v1.28.2+k3s1 (6330a5b)
go version go1.20.8

Node(s) CPU architecture, OS, and Version:

Linux ip-XXXX.us-west-2.compute.internal 5.14.0-375.el9.x86_64 #1 SMP PREEMPT_DYNAMIC Mon Oct 9 16:32:37 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Cluster Configuration:

Single control-plane node.

Describe the bug:

Expected helm CRDs to be not installed when

Steps To Reproduce:

  • Installed K3s:
    Relevant config
protect-kernel-defaults: true
disable-cloud-controller: true
disable-helm-controller: true
disable-network-policy: true
disable:
  - traefik
  - servicelb
  - helm-controller

Expected behavior:
Helm CRDs to not install when disable-helm-controller

Actual behavior:

kubectl api-resources | grep helm
helmchartconfigs                               helm.cattle.io/v1                      true         HelmChartConfig
helmcharts                                     helm.cattle.io/v1                      true         HelmChart

Additional context / logs:

@brandond
Copy link
Member

brandond commented Oct 19, 2023

The intent of this flag is to disable the controller on this node; it is not intended to change what CRDs are installed to the cluster. Ref:

types := append(helmcrd.List(), addoncrd.List()...)
factory.BatchCreateCRDs(ctx, types...)

We would probably entertain a PR to change this behavior, but I don't believe it is a bug. Is there some problem that this is causing, or were you just not expecting to see the CRDs when the controller is disabled?

@brandond brandond added this to the Backlog milestone Oct 19, 2023
@harsimranmaan
Copy link
Contributor Author

Thanks @brandond. It makes sense and I agree that it's not a bug but a slight nuance. I'll send over a PR.

@harsimranmaan
Copy link
Contributor Author

I am thinking of adding something like

disable-crds:
  - helm
  - addons

@brandond
Copy link
Member

We try to avoid adding more flags whenever possible. Also, you can't disable the addon CRDs as those support the deploy controller that cannot be disabled.

I would probably just keep it minimal, and modify that code to not add the helm CRDs to the list of CRDs to be created, when the helm controller is disabled.

harsimranmaan added a commit to harsimranmaan/k3s that referenced this issue Oct 19, 2023
The NewContext package requires config as input which would
require all third-party callers to update when the new go module
is published.

This change only affects the behaviour of installation of helm
CRDs. Existing helm crds installed in a cluster would not be removed
when disable-helm-controller flag is set on the server.

Addresses k3s-io#8701

Signed-off-by: Harsimran Singh Maan <maan.harry@gmail.com>
@brandond brandond modified the milestones: Backlog, v1.28.4+k3s1 Nov 14, 2023
@brandond brandond self-assigned this Nov 14, 2023
brandond pushed a commit that referenced this issue Nov 15, 2023
* Disable helm CRD installation for disable-helm-controller
    The NewContext package requires config as input which would
    require all third-party callers to update when the new go module
    is published.
    
    This change only affects the behaviour of installation of helm
    CRDs. Existing helm crds installed in a cluster would not be removed
    when disable-helm-controller flag is set on the server.
    
    Addresses #8701
* address review comments
* remove redundant check

Signed-off-by: Harsimran Singh Maan <maan.harry@gmail.com>
brandond pushed a commit to brandond/k3s that referenced this issue Nov 16, 2023
* Disable helm CRD installation for disable-helm-controller
    The NewContext package requires config as input which would
    require all third-party callers to update when the new go module
    is published.

    This change only affects the behaviour of installation of helm
    CRDs. Existing helm crds installed in a cluster would not be removed
    when disable-helm-controller flag is set on the server.

    Addresses k3s-io#8701
* address review comments
* remove redundant check

Signed-off-by: Harsimran Singh Maan <maan.harry@gmail.com>
(cherry picked from commit abc2efd)
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
brandond pushed a commit to brandond/k3s that referenced this issue Nov 16, 2023
* Disable helm CRD installation for disable-helm-controller
    The NewContext package requires config as input which would
    require all third-party callers to update when the new go module
    is published.

    This change only affects the behaviour of installation of helm
    CRDs. Existing helm crds installed in a cluster would not be removed
    when disable-helm-controller flag is set on the server.

    Addresses k3s-io#8701
* address review comments
* remove redundant check

Signed-off-by: Harsimran Singh Maan <maan.harry@gmail.com>
(cherry picked from commit abc2efd)
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
brandond pushed a commit to brandond/k3s that referenced this issue Nov 16, 2023
* Disable helm CRD installation for disable-helm-controller
    The NewContext package requires config as input which would
    require all third-party callers to update when the new go module
    is published.

    This change only affects the behaviour of installation of helm
    CRDs. Existing helm crds installed in a cluster would not be removed
    when disable-helm-controller flag is set on the server.

    Addresses k3s-io#8701
* address review comments
* remove redundant check

Signed-off-by: Harsimran Singh Maan <maan.harry@gmail.com>
(cherry picked from commit abc2efd)
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
brandond pushed a commit that referenced this issue Nov 16, 2023
* Disable helm CRD installation for disable-helm-controller
    The NewContext package requires config as input which would
    require all third-party callers to update when the new go module
    is published.

    This change only affects the behaviour of installation of helm
    CRDs. Existing helm crds installed in a cluster would not be removed
    when disable-helm-controller flag is set on the server.

    Addresses #8701
* address review comments
* remove redundant check

Signed-off-by: Harsimran Singh Maan <maan.harry@gmail.com>
(cherry picked from commit abc2efd)
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
brandond pushed a commit that referenced this issue Nov 16, 2023
* Disable helm CRD installation for disable-helm-controller
    The NewContext package requires config as input which would
    require all third-party callers to update when the new go module
    is published.

    This change only affects the behaviour of installation of helm
    CRDs. Existing helm crds installed in a cluster would not be removed
    when disable-helm-controller flag is set on the server.

    Addresses #8701
* address review comments
* remove redundant check

Signed-off-by: Harsimran Singh Maan <maan.harry@gmail.com>
(cherry picked from commit abc2efd)
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
brandond pushed a commit that referenced this issue Nov 16, 2023
* Disable helm CRD installation for disable-helm-controller
    The NewContext package requires config as input which would
    require all third-party callers to update when the new go module
    is published.

    This change only affects the behaviour of installation of helm
    CRDs. Existing helm crds installed in a cluster would not be removed
    when disable-helm-controller flag is set on the server.

    Addresses #8701
* address review comments
* remove redundant check

Signed-off-by: Harsimran Singh Maan <maan.harry@gmail.com>
(cherry picked from commit abc2efd)
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
@endawkins endawkins reopened this Nov 16, 2023
@endawkins
Copy link

Validated on branch <branch_name> with commit <commit_id> / version

Environment Details

Infrastructure

  • Cloud
  • Hosted

Node(s) CPU architecture, OS, and Version:

Linux ip-172-31-26-200 5.14.21-150400.24.60-default #1 SMP PREEMPT_DYNAMIC Wed Apr 12 12:13:32 UTC 2023 (93dbe2e) x86_64 x86_64 x86_64 GNU/Linux
NAME="SLES"
VERSION="15-SP4"
VERSION_ID="15.4"
PRETTY_NAME="SUSE Linux Enterprise Server 15 SP4"
ID="sles"
ID_LIKE="suse"
ANSI_COLOR="0;32"
CPE_NAME="cpe:/o:suse:sles:15:sp4"
DOCUMENTATION_URL="https://documentation.suse.com/"

Cluster Configuration:

1 server node

Config.yaml:

cluster-init: true
write-kubeconfig-mode: 644
protect-kernel-defaults: true
disable-cloud-controller: true
disable-helm-controller: true
disable-network-policy: true
disable:
  - traefik
  - servicelb
  - helm-controller

Additional files

N/A

Testing Steps

  1. Copy config.yaml
$ sudo mkdir -p /etc/rancher/k3s && sudo cp config.yaml /etc/rancher/k3s
  1. Install k3s
  2. Run the following command kubectl api-resources | grep helm

Replication Results:

  • k3s version used for replication:
k3s -v
k3s version v1.28.3+k3s2 (bbafb86e)
go version go1.20.10
kubectl api-resources | grep helm
helmchartconfigs                               helm.cattle.io/v1                      true         HelmChartConfig
helmcharts                                     helm.cattle.io/v1                      true         HelmChart

Validation Results:

  • k3s version used for validation:
k3s -v
k3s version v1.28.4-rc1+k3s1 (3f237230)
go version go1.20.11
ec2-user@<IP>:~> kubectl api-resources | grep helm
ec2-user@<IP>:~>

Returns a new line to show the Helm CRDS are not installed

Additional context / logs:

N/A

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done Issue
Development

Successfully merging a pull request may close this issue.

3 participants