[main] Add support for a FieldSelector in the ResourceSet#742
Conversation
There was a problem hiding this comment.
not a problem for this PR, but what do we do about clusters in a different namespace than fleet-default? cc @rancher/observation-backup
There was a problem hiding this comment.
yeah great question - those weren't being backed up before I would assume as well right? at least thats how this current ResourceSet reads.
There was a problem hiding this comment.
This is indeed a problem and there's an open issue for it here: #574
A workaround is to use the "resources.cattle.io/backup=true" label to make sure the secret is included in the backup.
charts/rancher-backup/files/default-resourceset-contents/provisioningv2.yaml
Outdated
Show resolved
Hide resolved
charts/rancher-backup/files/sensitive-resourceset-contents/provisioningv2.yaml
Outdated
Show resolved
Hide resolved
4fee098 to
5fd4256
Compare
5fd4256 to
3ea4287
Compare
|
Looks like @jakefhyde gave it an approval so I've assigned @jbiers for O&B side review before merge. Jake / @thatmidwesterncoder , is this something that is expected to backport to 2.11 (or other versions)? If yes, is it considered a critical issue that should be targeting 2.11.1? Forget if we covered that some where else but just want to verify the scope of this for backport considerations. (Essentially, because BRO hasn't branched after 2.11.0 release currently - if we want it in 2.11.1 we should merge first and then branch. If we only want this in 2.12.0 then we should branch first then merge this.) |
|
@mallardduck yes - we want it for 2.11.1 so we'll merge -> branch, we may want it in the 2.10 release line as well so I'll open a backport PR there after it gets merged. |
jbiers
left a comment
There was a problem hiding this comment.
Minor nitpick just to keep consistency in the codebase, once fix will be ready for approval
3ea4287 to
f2ca28e
Compare
There was a problem hiding this comment.
LGTM
It's best if we also get this issue tested by QA on our side as well. Should we have a separate issue or can I add rancher/rancher#48558 to my team's dashboard too? @thatmidwesterncoder
|
I think we may need 2 issues to track as with one, only one of the QA teams can mark it as done. I believe when the first team does that it would (likely) be hidden for the following team. Since their filters probably auto mark things as done if the issue is closed too. |
|
That sounds accurate. I assume our QAs would like to close this out too e.g. validating that the provisioning cluster objects are all present after restore. |
|
Created rancher/rancher#49647 to track the issue on our side |
|
Hopefully didn't pull the gun too early @thatmidwesterncoder / @jakefhyde ...this came up in our team sync up and with 2.11.1 looming we want to get this to QA ASAP. So I'm also preparing an RC and then making PS I also did the branching thing...so this change will be ready for 2.11.1 and 2.12.0; so we may need to consider back ports for 2.10 and 2.9 (maybe) now that this is moving forward. |
* add support for a FieldSelector in the ResourceSet * add cluster-state, machine-state, machine-plan to v2prov resource sets * add integration test for custom resource set including field selectors
* add support for a FieldSelector in the ResourceSet * add cluster-state, machine-state, machine-plan to v2prov resource sets * add integration test for custom resource set including field selectors
Issue: rancher/rancher#48558
Problem:
Certain very important secrets are not being backed up by the backup-restore operator when the cluster-name + pool-name gets to be longer than 63 characters, this is because they fail to match the v2prov regex.
Solution:
Add support for selecting resources via
fieldSelectorin order to be able to effectively filter on specific secret types that we care about. This will be useful for other "custom" secret types as well as being able to narrow things that have a certain label + fieldSelector.The solution is implemented by just tacking on a fieldSelector to the initial API listing if it is present. By default it selects everything mimicking old behavior (exactly like the label selector api).
Alternatives:
Testing:
I added e2e integration tests for this which fetch the tarball and extract it into memory - and then validate that the custom-resource-set was used and backed up a single secret (and not the other one)