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

1301 parametrize image locations #1705

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

danielpodwysocki
Copy link

@danielpodwysocki danielpodwysocki commented Feb 6, 2024

SUMMARY

This allows users of private registries to use Values.yml to configure the operator and rbac proxy images and point at their images, while keeping the default behaviour same as on the current chart.

It removes the need to constantly rebuild the chart in-house and makes it reusable across organizations.

It also includes a separate commit with a minor yq fix - a missing -y flag was causing it to error when testing the helm-chart make target.

ISSUE TYPE
  • New or Enhanced Feature
ADDITIONAL INFORMATION

Issue link:
#1301

Comparison of the resulting charts - ../../awx-operator is an unpacked official chart for 2.11.0:

➜  awx-operator git:(1301-parametrize-image-locations) diff ../../awx-operator/ charts/awx-operator
diff --color ../../awx-operator/Chart.yaml charts/awx-operator/Chart.yaml
2c2
< appVersion: 2.11.0
---
> appVersion: 0.1.0
6c6
< version: 2.11.0
---
> version: 2.11.0-5-g165d099
Common subdirectories: ../../awx-operator/crds and charts/awx-operator/crds
Common subdirectories: ../../awx-operator/templates and charts/awx-operator/templates
diff --color ../../awx-operator/values.yaml charts/awx-operator/values.yaml
0a1,10
> # images for the operator itself, not the AWX deployment
> # For airgapped installs, modify both those and the AWX.spec - you can pass the 
> # variables as defined in https://github.com/ansible/awx-operator/blob/devel/roles/installer/tasks/resources_configuration.yml
> operator_image: quay.io/ansible/awx-operator
> operator_version: latest
> 
> 
> rbac_proxy_image: gcr.io/kubebuilder/kube-rbac-proxy
> rbac_proxy_version: v0.15.0

yq requires passing -y for in-place edits and will error otherwise
@danielpodwysocki
Copy link
Author

For registries requiring authentication, this also needs a method to define imagePullSecrets.

I will update this PR when I get a moment to sort that.

@@ -15,7 +15,7 @@ spec:
capabilities:
drop:
- "ALL"
image: gcr.io/kubebuilder/kube-rbac-proxy:v0.15.0
image: "rbac_proxy"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielpodwysocki
Hi, I didn't make any test for this PR locally, but I think this would break non-helm based installation.
This image param will be used as-is in both make deploy and kubectl apply -k. ways that described in Basic install section and will cause ErrImagePull error: https://ansible.readthedocs.io/projects/awx-operator/en/latest/installation/basic-install.html
If you want to replace this image with rbac_proxy, further modifications should be made so that the default image is correctly specified for non-helm installations.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, wasn't aware you were also handling non-helm installs there - will circle back to this soon-ish, make it work and update the PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think to make this work for normal operator installs (non-helm), you would need to add an entry to this kustomization file:

that is similar to the existing one for the operator image, here:

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the PR to not break Kustomize - apologies it took a bit, life is sometimes very good at getting in the way of things :)

Would you mind having another look? I also made imagePullSecrets customizable (while keeping the default value).

Reading the code for the operator itself, the image_pull_secrets var defined in roles/installer/defaults/main.yml takes care of things already there, so that shouldn't need extra handling.

And thank you once more for having a look!

PS. quick test I did:

# deploying to a local dev cluster running in kind
➜  awx-operator git:(1301-parametrize-image-locations) ✗ make deploy
namespace/awx unchanged
customresourcedefinition.apiextensions.k8s.io/awxbackups.awx.ansible.com unchanged
customresourcedefinition.apiextensions.k8s.io/awxmeshingresses.awx.ansible.com unchanged
customresourcedefinition.apiextensions.k8s.io/awxrestores.awx.ansible.com unchanged
customresourcedefinition.apiextensions.k8s.io/awxs.awx.ansible.com unchanged
serviceaccount/awx-operator-controller-manager unchanged
role.rbac.authorization.k8s.io/awx-operator-awx-manager-role configured
role.rbac.authorization.k8s.io/awx-operator-leader-election-role unchanged
clusterrole.rbac.authorization.k8s.io/awx-operator-metrics-reader unchanged
clusterrole.rbac.authorization.k8s.io/awx-operator-proxy-role unchanged
rolebinding.rbac.authorization.k8s.io/awx-operator-awx-manager-rolebinding unchanged
rolebinding.rbac.authorization.k8s.io/awx-operator-leader-election-rolebinding unchanged
clusterrolebinding.rbac.authorization.k8s.io/awx-operator-proxy-rolebinding unchanged
configmap/awx-operator-awx-manager-config unchanged
service/awx-operator-controller-manager-metrics-service unchanged
deployment.apps/awx-operator-controller-manager unchanged
➜  awx-operator git:(1301-parametrize-image-locations) ✗ kubectl describe deployments.apps -n awx awx-operator-controller-manager  | grep -i image
    Image:      gcr.io/kubebuilder/kube-rbac-proxy:v0.15.0
    Image:      quay.io/ansible/awx-operator:2.11.0-7-g3d080f0

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

Successfully merging this pull request may close these issues.

None yet

3 participants