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

CRD's from subchart being deployed even when subchart dependencies.condition is false #8508

Closed
agniveshadhikari opened this issue Jul 23, 2020 · 13 comments
Labels
bug Categorizes issue or PR as related to a bug. Stale

Comments

@agniveshadhikari
Copy link

Using nginx ingress as a subchart. Even when I have the deployment condition resolve to false, it still tries to deploy the CRDs.

Output of helm version:

version.BuildInfo{Version:"v3.2.1", GitCommit:"fe51cd1e31e6a202cba7dead9552a6d418ded79a", GitTreeState:"clean", GoVersion:"go1.13.10"}

Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.0", GitCommit:"9e991415386e4cf155a24b1da15becaa390438d8", GitTreeState:"clean", BuildDate:"2020-03-25T14:58:59Z", GoVersion:"go1.13.8", Compiler:"gc", Platform:"windows/amd64"}
Server Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.5", GitCommit:"20c265fef0741dd71a66480e35bd69f18351daea", GitTreeState:"clean", BuildDate:"2019-10-15T19:07:57Z", GoVersion:"go1.12.10", Compiler:"gc", Platform:"linux/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.):
AKS

@agniveshadhikari agniveshadhikari changed the title CRD's from subchart being deployed even when subchart dependency.condition is false CRD's from subchart being deployed even when subchart dependencies.condition is false Jul 23, 2020
@bacongobbler
Copy link
Member

hi @agniveshadhikari. Can you please provide a test case to reproduce the issue. Without a test case, we cannot ensure that we are asserting the same behaviour you described here.

@agniveshadhikari
Copy link
Author

agniveshadhikari commented Jul 30, 2020

Hi @bacongobbler, please checkout this repo and try out helm install --debug. You should get output similar to

client.go:108: [debug] creating 1 resource(s)
install.go:140: [debug] CRD aplogconfs.appprotect.f5.com is already present. Skipping.
client.go:108: [debug] creating 1 resource(s)
install.go:140: [debug] CRD appolicies.appprotect.f5.com is already present. Skipping.
client.go:108: [debug] creating 1 resource(s)
install.go:140: [debug] CRD globalconfigurations.k8s.nginx.org is already present. Skipping.
client.go:108: [debug] creating 1 resource(s)
install.go:140: [debug] CRD policies.k8s.nginx.org is already present. Skipping.
client.go:108: [debug] creating 1 resource(s)
install.go:140: [debug] CRD transportservers.k8s.nginx.org is already present. Skipping.
client.go:108: [debug] creating 1 resource(s)
install.go:140: [debug] CRD virtualservers.k8s.nginx.org is already present. Skipping.
client.go:108: [debug] creating 1 resource(s)
install.go:140: [debug] CRD virtualserverroutes.k8s.nginx.org is already present. Skipping.

The condition resolves to false, so it doesn't render/install the templates from the subchart, but the CRDs are still traversed.

@agniveshadhikari
Copy link
Author

Bump! @bacongobbler Did you get a chance to try this out?

@bacongobbler
Copy link
Member

I don’t have enough time to test this unfortunately. Let us know what you find out.

@fbartnitzek
Copy link

Bump - any news on this one?

@hickeyma
Copy link
Contributor

hickeyma commented Oct 12, 2020

@agniveshadhikari I did a test using the latest from the master branch. I created a scaffold chart (issue-8505-sub1), removed its templates and added a simple CRD and CR (ref https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/). I then created a second scaffold chart (issue-8505-sub1) which adds the first chart as a dependency. The output shows that I can do a --dry-run when thecron.enabled is false and the subchart and its CRD is not rendered. When I then set thecron.enabled to true it is then rendered and an error is shown because CRDs cannot be rendered during a --dry-run as explained in the CRD doc. This seems to show that the condition flag is working as expected.

$ helm create issue-8505-sub1
Creating issue-8505-sub1

$ cat issue-8505-sub1/crds/CrontabCustomRes.yaml 
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  # name must match the spec fields below, and be in the form: <plural>.<group>
  name: crontabs.stable.example.com
[...]

$ cat issue-8505-sub1/templates/crontab.yaml 
apiVersion: "stable.example.com/v1"
kind: CronTab
metadata:
  name: my-new-cron-object
spec:
  cronSpec: "* * * * */5"
  image: my-awesome-cron-image

$ helm create issue-8505
Creating issue-8505

$ cat issue-8505/Chart.yaml 
apiVersion: v2
name: issue-8505
description: A Helm chart for Kubernetes
[...]
dependencies:
- name: issue-8505-sub1
  repository: file://../issue-8505-sub1
  version: 0.1.0
  condition: thecron.enabled

$ helm dep update issue-8505
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "jetstack" chart repository
...Successfully got an update from the "ibm-stable" chart repository
...Successfully got an update from the "stable" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 1 charts
Deleting outdated charts

$ ls issue-8505/charts/
issue-8505-sub1-0.1.0.tgz

$ helm install issue-8505 issue-8505/ --dry-run --debug
install.go:172: [debug] Original chart version: ""
install.go:189: [debug] CHART PATH: /home/marty/test/helm-charts/issue-8505

install.go:189: [debug] WARNING: This chart or one of its subcharts contains CRDs. Rendering may fail or contain inaccuracies.
NAME: issue-8505
LAST DEPLOYED: Mon Oct 12 11:04:27 2020
NAMESPACE: default
STATUS: pending-install
REVISION: 1
USER-SUPPLIED VALUES:
{}

COMPUTED VALUES:
affinity: {}
[...]
thecron:
  enabled: false
tolerations: []

HOOKS:
---
# Source: issue-8505/templates/tests/test-connection.yaml
[...]
---
# Source: issue-8505/templates/serviceaccount.yaml
[...]
---
# Source: issue-8505/templates/service.yaml
[...]
---
# Source: issue-8505/templates/deployment.yaml
[...]

NOTES:
1. Get the application URL by running these commands:
  export POD_NAME=$(kubectl get pods --namespace default -l "app.kubernetes.io/name=issue-
[...]

$ kubectl get crd --all-namespaces
No resources found

$ helm install issue-8505 issue-8505/ --dry-run --debug --set thecron.enabled=true
install.go:172: [debug] Original chart version: ""
install.go:189: [debug] CHART PATH: /home/marty/test/helm-charts/issue-8505

install.go:189: [debug] WARNING: This chart or one of its subcharts contains CRDs. Rendering may fail or contain inaccuracies.
Error: unable to build kubernetes objects from release manifest: unable to recognize "": no matches for kind "CronTab" in version "stable.example.com/v1"
helm.go:81: [debug] unable to recognize "": no matches for kind "CronTab" in version "stable.example.com/v1"
unable to build kubernetes objects from release manifest
helm.sh/helm/v3/pkg/action.(*Install).Run
	/home/marty/go/src/helm.sh/helm/pkg/action/install.go:257
main.runInstall
	/home/marty/go/src/helm.sh/helm/cmd/helm/install.go:241
main.newInstallCmd.func2
	/home/marty/go/src/helm.sh/helm/cmd/helm/install.go:120
github.com/spf13/cobra.(*Command).execute
	/home/marty/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:842
github.com/spf13/cobra.(*Command).ExecuteC
	/home/marty/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:950
github.com/spf13/cobra.(*Command).Execute
	/home/marty/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:887
main.main
	/home/marty/go/src/helm.sh/helm/cmd/helm/helm.go:80
runtime.main
	/usr/local/go/src/runtime/proc.go:203
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1373

$ kubectl get crd --all-namespaces
No resources found

@hickeyma
Copy link
Contributor

hickeyma commented Oct 12, 2020

I then decided to install the parent chart (without --dry-run flag) with condition false again and it installed the chart as expected:

$ helm install issue-8505 issue-8505/
NAME: issue-8505
LAST DEPLOYED: Mon Oct 12 11:57:24 2020
NAMESPACE: default
STATUS: deployed
REVISION: 1
NOTES:
1. Get the application URL by running these commands:
  export POD_NAME=$(kubectl get pods --namespace default -l "app.kubernetes.io/name=issue-8505,app.kubernetes.io/instance=issue-8505" -o jsonpath="{.items[0].metadata.name}")
  export CONTAINER_PORT=$(kubectl get pod --namespace default $POD_NAME -o jsonpath="{.spec.containers[0].ports[0].containerPort}")
  echo "Visit http://127.0.0.1:8080 to use your application"
  kubectl --namespace default port-forward $POD_NAME 8080:$CONTAINER_PORT

$ helm3 ls
NAME      	NAMESPACE	REVISION	UPDATED                                	STATUS  	CHART           	APP VERSION
issue-8505	default  	1       	2020-10-12 11:57:24.353692965 +0000 UTC	deployed	issue-8505-0.1.0	1.16.0     

However, it also installed the CRD:

$ kubectl get crd --all-namespaces
NAME                          CREATED AT
crontabs.stable.example.com   2020-10-12T11:57:22Z

I am going to set to a bug as it should not install the CRD when condition is false.

@hickeyma hickeyma added bug Categorizes issue or PR as related to a bug. and removed question/support labels Oct 12, 2020
@hasheddan
Copy link

We have encountered this bug in https://github.com/crossplane/crossplane where we have a dependency that installs v1 CRDs. We have a flag to gate installing this dependency so that folks using pre-v1.16 k8s clusters can still install Crossplane, but this bug breaks that compatibility. I would be happy to help out with getting this fixed. Thank you for the awesome work y'all do!

@github-actions
Copy link

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

@Demonsthere
Copy link

Bump for interest

@ramanNarasimhan77
Copy link

Hello
Can this issue be re-opened as this issue seems to be happening even now?

@jackson-chris
Copy link

This is still a problem. Please fix.

@zak905
Copy link
Contributor

zak905 commented Jul 19, 2023

Hi, any updates about this issue ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Categorizes issue or PR as related to a bug. Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants