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

Add overlay for deployment #20

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Add overlay for deployment #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .helm/starter/templates/operator-controller/_operator-controller.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{{/*
This template gets filled with the operator SDK's controller definition
during chart build
*/}}
{{- define "awx-operator.controller" -}}
4 changes: 4 additions & 0 deletions .helm/starter/templates/operator-controller/controller-overlay.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{{- $operatorControllerTemplate := (include "awx-operator.controller" $) | fromYaml }}
{{- $_ := mergeOverwrite $operatorControllerTemplate (index $.Values "operator-controller") }}

{{ $operatorControllerTemplate | toYaml }}
2 changes: 2 additions & 0 deletions .helm/starter/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ AWX:
password: Unset
sslmode: prefer
type: unmanaged

controller: {}
15 changes: 9 additions & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,21 +151,24 @@ helm-chart-generate: kustomize helm kubectl-slice yq charts
for file in charts/$(CHART_NAME)/raw-files/*rolebinding*; do\
$(YQ) -i '.subjects[0].namespace = "{{ .Release.Namespace }}"' $${file};\
done
# Add .spec.replicas for the controller-manager deployment
for file in charts/$(CHART_NAME)/raw-files/deployment-*-controller-manager.yaml; do\
$(YQ) -i '.spec.replicas = "{{ (.Values.Operator).replicas | default 1 }}"' $${file};\
done

# Correct .metadata.name for cluster scoped resources
cluster_scoped_files="charts/$(CHART_NAME)/raw-files/clusterrolebinding-awx-operator-proxy-rolebinding.yaml charts/$(CHART_NAME)/raw-files/clusterrole-awx-operator-metrics-reader.yaml charts/$(CHART_NAME)/raw-files/clusterrole-awx-operator-proxy-role.yaml";\
for file in $${cluster_scoped_files}; do\
$(YQ) -i '.metadata.name += "-{{ .Release.Name }}"' $${file};\
done
# Correct the reference for the clusterrolebinding
$(YQ) -i '.roleRef.name += "-{{ .Release.Name }}"' 'charts/$(CHART_NAME)/raw-files/clusterrolebinding-awx-operator-proxy-rolebinding.yaml'
# Correct .spec.replicas type for the controller-manager deployment

# Feed controller deployment file into template to allow for override from values
for file in charts/$(CHART_NAME)/raw-files/deployment-*-controller-manager.yaml; do\
$(SED_I) "s/'{{ (.Values.Operator).replicas | default 1 }}'/{{ (.Values.Operator).replicas | default 1 }}/g" $${file};\
cat $${file} >> charts/$(CHART_NAME)/templates/operator-controller/_operator-controller.tpl;\
echo "\n---" >> charts/$(CHART_NAME)/templates/operator-controller/_operator-controller.tpl;\
rm -f $${file} ;\
done
echo '{{- end -}}' >> charts/$(CHART_NAME)/templates/operator-controller/_operator-controller.tpl


# move all custom resource definitions to crds folder
mkdir charts/$(CHART_NAME)/crds
mv charts/$(CHART_NAME)/raw-files/customresourcedefinition*.yaml charts/$(CHART_NAME)/crds/.
Expand Down
34 changes: 23 additions & 11 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Use `--force-conflicts` flag to resolve the conflict.
kubectl apply --server-side --force-conflicts -k github.com/ansible/awx-operator/config/crd?ref=<VERSION>
```

## Configuration
## Custom Resource Configuration

The goal of adding helm configurations is to abstract out and simplify the creation of multi-resource configs. The `AWX.spec` field maps directly to the spec configs of the `AWX` resource that the operator provides, which are detailed in the [main README](https://github.com/ansible/awx-operator/blob/devel/README.md). Other sub-config can be added with the goal of simplifying more involved setups that require additional resources to be specified.

Expand Down Expand Up @@ -246,6 +246,28 @@ AWX:

## Values Summary

### Controller
The configuration of the `awx-operator-controller-manager` `Deployment` resource can be overridden by the
`operator-controller` field. Any fields specified under this key will map directly onto the root hierarchy of the [Deployment](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/) configuration.

For example, to override the replicas of the controller deployment, use:
Copy link
Collaborator

@schen1 schen1 Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

While the example of the replica is valid. This is not really a common scenario to customize the operator replica (maybe to scale up/scale down). One more common use case is to specify a different container image for each container as seen in this issue or that one to cover disconnected/restricted environments.
Another useful example would be to provide an example for setting up resource requests and limits, this would cover this open issue.

What do you think? Could you maybe provide an example on how to do that @miles-w-3

Thanks for the PR btw!

Copy link
Contributor

Choose a reason for hiding this comment

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

@schen1 Perhaps we should capture these items in a follow up to improve the docs. While I think extensive documentation is always a good thing, I'm not sure we should hold up this PR much longer. Any thoughts @miles-w-3 ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

fine for me :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am fine either way. The use case for overriding an image tag is tricky because of the way lists are merged in helm.

Lists are overridden instead of merged for each layer. So under this proposal, if you want to change the image version, you have to override the entire container list spec. For that reason, this solution is more optimal for adding things like annotations or service account specs to the controller's pods.

If there is desire for more targeted overrides of specific containers, that could be another feature for a template that iterates through a list, matches a container name, and then merges a set override

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the details and the great contribution. Then it makes sense to have it as a different feature/PR


```yml
# values
operator-controller:
spec:
replicas: 4
```

Similarly, to add or override annotations:
```yml
#values
operator-controller:
metadata:
annotations:
my-key: my-value
```

### AWX

| Value | Description | Default |
Expand All @@ -263,12 +285,6 @@ AWX:
|---|---|---|
| `extraDeploy` | array of additional resources to be deployed (supports YAML or literal "\|") | - |

### Operator

| Value | Description | Default |
|---|---|---|
| `Operator.replicas` | Number of controller-manager instance replicas | `1` |

### customSecrets

| Value | Description | Default |
Expand Down Expand Up @@ -400,10 +416,6 @@ Where possible, defer to `AWX.spec` configs before applying the abstracted confi

This chart is built using the Makefile in the [awx-operator repo](https://github.com/ansible/awx-operator). Clone the repo and run `make helm-chart`. This will create the awx-operator chart in the `charts/awx-operator` directory. In this process, the contents of the `.helm/starter` directory will be added to the chart.

### Future Goals

All values under the `AWX` header are focused on configurations that use the operator. Configurations that relate to the Operator itself could be placed under an `Operator` heading, but that may add a layer of complication over current development.

## Chart Publishing

The chart is currently hosted on the gh-pages branch of the repo. During the release pipeline, the `index.yaml` stored in that branch is generated with helm chart entries from all valid tags. We are currently unable to use the `chart-releaser` pipeline due to the fact that the complete helm chart is not committed to the repo and is instead built during the release process. Therefore, the cr action is unable to compare against previous versions.
Expand Down