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

Deprecate pulp_settings in favor of extra_settings #100

Closed
wants to merge 12 commits into from

Conversation

Denney-tech
Copy link

@Denney-tech Denney-tech commented Apr 10, 2024

SUMMARY

I think it would make sense for the galaxy-operator to behave more like the awx-operator for consistency's sake. This adds extra_settings: to the CRD and the settings.py.j2 template. I didn't see any data manipulation in the awx-operator roles, so I presume none are needed here.

ADDITIONAL INFORMATION

forum.ansible.com thread

Copied and modified documentation from awx-operator's extra_settings to here.

Depends on #98

@rooftopcellist
Copy link
Member

@Denney-tech I definitely want something like this. But there are a couple changes I think we'll need:

  • We should mark the existing pulp_settings parameter as "Deprecated" in the CRD and CSV. But we can't remove it because that would cause CRD schema conflicts within the same apiVersion (tl;dr, nightmares for upgrades)
  • We will also need the templating logic to have backwards compatibility for pulp_settings.

@Denney-tech
Copy link
Author

I added a deprecation warning in an assertion step, but not sure what to do about the CRD/CSV.

This is backwards compatible with #98, and I wasn't intending to remove pulp_settings entirely. I think pulp_settings should stay for a handful of releases and maybe set a target removal date.

@Denney-tech
Copy link
Author

So, just to clarify since I'm a little green when it comes to k8s operators; we can't remove pulp_settings (or any other parameter) from a CRD unless we change the version, such as: galaxy_v1beta1_galaxy_crd.yaml#L15?

Does the CSV have to match the CRD? I don't see a similar version spec in the CSV.

@Denney-tech
Copy link
Author

@rooftopcellist fixed the crd error, a mistake in the csv, and resolved the merge conflict that was present.

@Denney-tech
Copy link
Author

Muddied the water a bit in my fork, so I'm closing these PR's while I cleanup.

@Denney-tech Denney-tech deleted the extra_settings branch May 14, 2024 18:01
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

2 participants