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

Add postgres init container to resolve permissions for some k3s deployments #1805

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

rooftopcellist
Copy link
Member

@rooftopcellist rooftopcellist commented Apr 2, 2024

Alternate PR for #1799

Resolves:

This approach allows users to simply set:

spec:
  postgres_data_volume_init: true

This removes the unused postgres_init_container_resource_requirements parameter which was never removed when the last postgres init container was removed. I cannot think of a good reason to not just use postgres_resource_requirements.

cc @kurokobo

ISSUE TYPE
  • New or Enhanced Feature

@rooftopcellist rooftopcellist changed the title Add postgres init container Add postgres init container to resolve permissions for some k3s deployments Apr 2, 2024
Add postgres init container if
postgres_data_volume_init is true

This allows users to run arbitrary commands

This is aimed to solve the issue where users may
need to chmod or chown the postgres
data volume for user 26, which is the user
that is running postgres in the sclorg image.

For example, one can now set the follow on the AWX spec:
spec:
  postgres_init_container_commands: |
    chown 26:0 /var/lib/pgsql/data
    chmod 700 /var/lib/pgsql/data

In addition, remove the unneeded
postgres_init_container_resource_requirements parameter

Signed-off-by: Seth Foster <fosterbseth@gmail.com>
Co-authored-by: Christian M. Adams <chadams@redhat.com>
@rooftopcellist
Copy link
Member Author

@kurokobo I just realized that in your awx-on-k3s repo, there are a couple places where you reference postgres_init_container_resource_requirements

image

If a user has this specified on their AWX spec on upgrade or fresh install, they will get this error:

Error from server (BadRequest): error when creating "base": AWX in version "v1beta1" cannot be handled as a AWX: strict decoding error: unknown field "spec.postgres_init_container_resource_requirements"

It's probably safer for me to just add back in that parameter and mark it as deprecated, and add a backwards compat shim. I'll add that now.

…w param

Deprecate postgres_init_container_resource_requirements param in favor
of postgres_resource_requirements param

Co-authored-by: craph <14820052+craph@users.noreply.github.com>
Co-authored-by: kurokobo <kuro664@gmail.com>
@rooftopcellist
Copy link
Member Author

I just added the changes mentioned so that the validator does not error if the user has postgres_init_container_resource_requirements defined. It also accepts that parameter and uses it preferentially over postgres_resource_requirements for the init container only if provided. Though I marked it as deprecated so that we can eventually remove it as there is no real need to have two separate variables for this imo.

Testing

I tested this out with the awx-on-k3s repo with the follow AWX spec:

---
apiVersion: awx.ansible.com/v1beta1
kind: AWX
metadata:
  name: awx
spec:
  # all of the defaults from https://github.com/kurokobo/awx-on-k3s/blob/main/base/awx.yaml

  postgres_init_container_resource_requirements: {}
  postgres_data_volume_init: true

I confirmed that the deployment came up successfully, and that the pg init param works:

$ kubectl -n awx get pod awx-postgres-15-0 -o yaml | grep resources: -B1
...
    name: init
    resources: {}

Then also tested with:

---
apiVersion: awx.ansible.com/v1beta1
kind: AWX
metadata:
  name: awx
spec:
  # all of the defaults from https://github.com/kurokobo/awx-on-k3s/blob/main/base/awx.yaml

  # postgres_init_container_resource_requirements: {} #commented out
  postgres_data_volume_init: true
  postgres_init_container_commands: |
    chown 26:0 /var/lib/pgsql/data
    chmod 700 /var/lib/pgsql/data

I confirmed that the deployment came up successfully.

Finally, I did one more with postgres_resource_requirements set to something custom just to make sure there were no regressions introduced there.

spec:
  postgres_resource_requirements:
    requests:
      cpu: 100m
      memory: 256Mi
    limits:
      cpu: 500m
      memory: 512Mi

After the deployment finished, I checked that the requests and limits were set correctly.

$ kubectl -n awx get pod awx-postgres-15-0 -o yaml | grep resources: -B1 -A6
      protocol: TCP
    resources:
      limits:
        cpu: 500m
        memory: 512Mi
      requests:
        cpu: 100m
        memory: 256Mi
--
    name: init
    resources:
      limits:
        cpu: 500m
        memory: 512Mi
      requests:
        cpu: 100m
        memory: 256Mi

@rooftopcellist rooftopcellist merged commit a5211fe into ansible:devel Apr 3, 2024
7 checks passed
@rooftopcellist
Copy link
Member Author

As follow-up I opened a PR to update the main branch of the awx-on-k3s repo cc @kurokobo

@kurokobo
Copy link
Contributor

kurokobo commented Apr 4, 2024

@rooftopcellist

It's probably safer for me to just add back in that parameter and mark it as deprecated, and add a backwards compat shim. I'll add that now.

Thanks for the confirmation and sending PR, my guide is only unofficial one, so I think my guide should respect the Operator's implementation, and I don't think the Operator needs to adapt my guide. But yes, just marking the param as deprecated is safer. Thanks!

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.

None yet

4 participants