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

[BUG] Our reconciliation logic is not handling a field removal from CR #613

Open
git-hyagi opened this issue Sep 19, 2022 · 2 comments
Open
Labels
bug Something isn't working go-alpha Go-based operator issues prio-list Issue with high priority and no assigners

Comments

@git-hyagi
Copy link
Collaborator

Our reconciliation logic is not handling a resource removal from CR, it is only checking if what is defined is deployed.

@git-hyagi git-hyagi added bug Something isn't working Triage-Needed go-alpha Go-based operator issues prio-list Issue with high priority and no assigners and removed Triage-Needed labels Sep 19, 2022
@git-hyagi git-hyagi self-assigned this Sep 26, 2022
@git-hyagi
Copy link
Collaborator Author

After an internal discussion, we decided that, for now, if a field is removed we will use mutating webhooks to set it back to the same value as before.

This is to avoid issues like removing a PVC field and losing all the data (if no PVC is defined, the reconcile logic would define an emptyDir).

If we find that this is not the best approach, we will tweak the settings for each field of Pulp CR.

git-hyagi added a commit to git-hyagi/pulp-operator that referenced this issue Oct 4, 2022
@git-hyagi
Copy link
Collaborator Author

git-hyagi commented Oct 4, 2022

We did some tests and could make the validation through the admission controller work, but decided to not enable it yet.

To enable the validation webhooks users would need to first configure a certificate that needs to be trusted by api pods.
We checked some ways to achieve this:

  • through service certificate in OCP environments
  • by manually installing cert-manager
  • by manually installing cert-manager-operator in OCP environments
  • by manually creating and configuring the certificates and CA

Considering that any of the above alternatives would be another burden for the operator adoption, we decided to not merge #658 into main yet.

@git-hyagi git-hyagi removed their assignment Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go-alpha Go-based operator issues prio-list Issue with high priority and no assigners
Projects
Archived in project
Development

No branches or pull requests

1 participant