Open StopMotionCuber opened 2 months ago
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: StopMotionCuber Once this PR has been reviewed and has the lgtm label, please assign dkliban for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Hi @StopMotionCuber. Thanks for your PR.
I'm waiting for a pulp member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test
on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test
label.
I understand the commands that are listed here.
Just as a side-note: here a non-squashed version of the PR.
I'm not sure about the CI failures, whether these are just flaky or whether there is an actual error. Logs are not that great for the failures
This lifts the restriction of fields being immutable in many places without a good reason for that behavior.
The
DeploymentType
is still mutable, as I didn't figure out completely what that was about and assume that changing that betweenpulp
andgalaxy
has deeper implications, rendering the instance useless (in that case newly creating an instance is indeed the way to go). As suggested by @git-hyagi here, I've added a transition hook which should return a failure instead of silently rolling back the value on clusters where supported. If that assumption is false and changing betweenpulp
andgalaxy
is possible without bad implications, I could remove the whole rollback machinery.I've tested the changes on a local k3s cluster with following scenarios:
settings.py
afterwards had the values of the second S3 secret included and the deployments were restarted.admin_password_secret
with a new one (where no secret existed). A new secret with new credentials was created and I was able to login with these credentials, while not being able to login with the former credentials. Former credentials were not deleted, but I guess that's fine.I didn't test all of the secrets, as I do not have any automation for that, but I think all of them are in the same code path, updating the
pulp-server:settings.py
, which should all trigger proper reconciliation and restart of the services.Closes #1343