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

remove namespace from helm chart #954

Merged
merged 1 commit into from
Jul 11, 2022
Merged

Conversation

bewing
Copy link
Contributor

@bewing bewing commented Jun 19, 2022

Helm should be able to set the namespace for the operator at deploy time
via the --namespace option. Use yq to remove all references to
namespaces in the helm chart prior to publishing.

Resolves #907

@bewing
Copy link
Contributor Author

bewing commented Jun 19, 2022

helm upgrade --install --namespace foo foo .
WARNING: Kubernetes configuration file is group-readable. This is insecure. Location: /home/nicotine/.kube/config
WARNING: Kubernetes configuration file is world-readable. This is insecure. Location: /home/nicotine/.kube/config
Release "foo" does not exist. Installing it now.
Error: ClusterRoleBinding.rbac.authorization.k8s.io "awx-operator-proxy-rolebinding" is invalid: subjects[0].namespace: Required value

More thought needed.

@bewing
Copy link
Contributor Author

bewing commented Jun 19, 2022

New changeset resolves issues. Was able to install without issue.

$ make docker-build
$ make helm-chart
$ helm install --namespace arbitrary --create-namespace awx-operator charts/awx-operator
$ kubectl get pods --namespace arbitrary
NAME                                               READY   STATUS    RESTARTS   AGE
awx-operator-controller-manager-66d7c6b944-dprp9   2/2     Running   0          36s
$ kubectl apply -f awx-demo.yml --namespace arbitrary
awx.awx.ansible.com/awx-demo created
$ kubectl get pods --namespace arbitrary
NAME                                               READY   STATUS    RESTARTS   AGE
awx-demo-778969fdf5-nqnn8                          4/4     Running   0          8s
awx-demo-postgres-0                                1/1     Running   0          20s
awx-operator-controller-manager-66d7c6b944-dprp9   2/2     Running   0          2m56s

@janorn
Copy link
Contributor

janorn commented Jun 21, 2022

This is exactly what we need!

@shanemcd
Copy link
Member

Thanks for this. Can you take a look and see why the tests are failing?

@bewing
Copy link
Contributor Author

bewing commented Jun 23, 2022

Thanks for this. Can you take a look and see why the tests are failing?

Updated Github CI to create the namespace during helm install, due to removed namespace yaml files in chart.

@shanemcd
Copy link
Member

Looks like the tests are still failing with Error: INSTALLATION FAILED: namespaces "awx" not found - might be faster for you to iterate on a PR against your own fork.

@miles-w-3
Copy link
Contributor

I think the problem is that, while --create-namespace is specified, the namespace itself isn't. Adding -n awx might do the trick

@miles-w-3
Copy link
Contributor

@shanemcd, adding -n awx to make the line:

helm install --wait my-awx-operator --create-namespace -n awx ./charts/awx-operator

worked, do you have the authority to modfiy this PR's branch? Here's the ci output from my fork when adding that arg:
https://github.com/miles-w-3/awx-operator/runs/7063487472?check_suite_focus=true

Helm should be able to set the namespace for the operator at deploy time
via the --namespace option.  Use yq to remove all references to
namespaces in the helm chart prior to publishing.

Update CI process to create namespace during install.

Resolves ansible#907
@bewing
Copy link
Contributor Author

bewing commented Jun 27, 2022

Added namespace specifier to CI. Rebased. CI passes in fork: https://github.com/bewing/awx-operator/runs/7078076867?check_suite_focus=true

@bewing
Copy link
Contributor Author

bewing commented Jul 6, 2022

Is there anything else needed before we run the workflow again, pending approval?

@shanemcd shanemcd merged commit c5db0e7 into ansible:devel Jul 11, 2022
@bewing
Copy link
Contributor Author

bewing commented Jul 14, 2022

Was this change included in the 0.24.0 release? The packaged chart doesn't appear to have this fix:

$ helm upgrade --install awx-operator --repo https://ansible.github.io/awx-operator/ --namespace arbitrary --create-namespace --version 0.24.0 awx-operator
Error: UPGRADE FAILED: failed to create resource: namespaces "awx" not found

@bewing
Copy link
Contributor Author

bewing commented Jul 14, 2022

Maybe the CI was using the previous version of the Makefile? Looking at logs now

@dvaerum
Copy link

dvaerum commented Jul 14, 2022

I am also not seeing it work when I am trying

@bewing
Copy link
Contributor Author

bewing commented Jul 14, 2022

Looking at https://github.com/ansible/awx-operator/runs/7307244861?check_suite_focus=true#step:7:26

It does delete the namespace.yaml file, but I don't see it running the macro to expand the files to yq for namespace removal. Does anyone know what make is used in the Github action ubuntu-latest VM?

Even weirder: https://github.com/ansible/awx-operator/runs/7307244861?check_suite_focus=true#step:7:32
make helm-index depends on helm-chart, and it looks like it DID run the macros and remove the namespaces there. What's causing the non-determinism here?

@bewing
Copy link
Contributor Author

bewing commented Jul 14, 2022

I think I see what the problem is -- the macro is only evaluated when the makefile is first read.

@bewing
Copy link
Contributor Author

bewing commented Jul 14, 2022

Addressed in #984

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

Successfully merging this pull request may close these issues.

awx-operator installation issue with helm.
5 participants