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

Skip restore of APIServices managed by Kubernetes #4028

Conversation

zubron
Copy link
Contributor

@zubron zubron commented Aug 10, 2021

Please add a summary of your change

It was discovered during Velero 1.6.3 upgrade testing that Velero was
restoring APIService objects for APIs that are no longer being served
by Kubernetes 1.22. If these items were restored, it would break the
behaviour of discovery within the cluster.

This change introduces a new RestoreItemAction plugin that skips the
restore of any APIService object which is managed by Kubernetes such
as those for built-in APIs or CRDs. The APIServices for these will be
created when the Kubernetes API server starts or when new CRDs are
registered. These objects are identified by looking for the
kube-aggregator.kubernetes.io/automanaged label.

Signed-off-by: Bridget McErlean [email protected]

Does your change fix a particular issue?

Fixes #4027

Please indicate you've done the following:

It was discovered during Velero 1.6.3 upgrade testing that Velero was
restoring `APIService` objects for APIs that are no longer being served
by Kubernetes 1.22. If these items were restored, it would break the
behaviour of discovery within the cluster.

This change introduces a new RestoreItemAction plugin that skips the
restore of any `APIService` object which is managed by Kubernetes such
as those for built-in APIs or CRDs. The `APIService`s for these will be
created when the Kubernetes API server starts or when new CRDs are
registered. These objects are identified by looking for the
`kube-aggregator.kubernetes.io/automanaged` label.

Signed-off-by: Bridget McErlean <[email protected]>
@zubron zubron force-pushed the add-restore-item-action-to-skip-automanaged-apiservices branch from 75e384f to 984176f Compare August 10, 2021 22:22
@zubron zubron added this to the v1.6.3 milestone Aug 10, 2021
@github-actions github-actions bot requested review from sseago and ywk253100 August 10, 2021 22:36
@github-actions github-actions bot added Dependencies Pull requests that update a dependency file has-changelog has-unit-tests labels Aug 10, 2021
@zubron zubron requested a review from dsu-igeek August 10, 2021 22:41
sseago
sseago previously approved these changes Aug 11, 2021
Copy link
Contributor

@dsu-igeek dsu-igeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the label functionality in ResourceSelector if it works.

pkg/restore/apiservice_action_test.go Outdated Show resolved Hide resolved
output := velero.NewRestoreItemActionExecuteOutput(input.Item)

if _, ok := apiService.Labels[autoregister.AutoRegisterManagedLabel]; ok {
output = output.WithoutRestore()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should log that it's being skipped

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have logging in the outer scope that state that the item is being skipped due to a plugin but will add something here to explain why 👍

pkg/restore/apiservice_action.go Show resolved Hide resolved
@zubron zubron force-pushed the add-restore-item-action-to-skip-automanaged-apiservices branch 2 times, most recently from 231789a to a64ceca Compare August 11, 2021 20:51
@zubron zubron requested review from dsu-igeek and sseago August 11, 2021 20:53
Instead of converting the unstructured item to check for the presence of
the `kube-aggregator.kubernetes.io/automanaged` label, use this label in
the `AppliesTo` to enable the restore logic to select the item. This
means that any item that matches the selector will have restore skipped.

Also add a new test case to the restore action test to check that label
selectors are applied correctly.

Signed-off-by: Bridget McErlean <[email protected]>
@zubron zubron force-pushed the add-restore-item-action-to-skip-automanaged-apiservices branch from a64ceca to 368098b Compare August 11, 2021 21:33
Copy link
Contributor

@dsu-igeek dsu-igeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@sseago sseago merged commit bf60621 into vmware-tanzu:main Aug 12, 2021
zubron pushed a commit to zubron/velero that referenced this pull request Aug 12, 2021
…ion-to-skip-automanaged-apiservices

Skip restore of APIServices managed by Kubernetes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dependencies Pull requests that update a dependency file has-changelog has-unit-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Velero restores APIService objects for APIs that are no longer served by Kubernetes 1.22
3 participants