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

Removing Snapshot tester #231

Merged
merged 6 commits into from
Mar 13, 2024

Conversation

mikemcd3912
Copy link
Contributor

Issue #, if available:
N/A
Description of changes:
Removing the snapshot tester from Baremetal to remedy hardcoded storage class issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mikemcd3912 mikemcd3912 requested a review from elamaran11 March 13, 2024 15:20
Copy link
Contributor

@elamaran11 elamaran11 left a comment

Choose a reason for hiding this comment

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

@mikemcd3912 test-app can still remain, we want to see if the CSI driver is creating a PVC. You can remove snapshot, snapshot-restore which is not supported in the CSI driver

Copy link
Contributor

@elamaran11 elamaran11 left a comment

Choose a reason for hiding this comment

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

@mikemcd3912 One more feedback, Also test this pls.

@@ -1,4 +0,0 @@
apiVersion: kustomize.config.k8s.io/v1beta1
Copy link
Contributor

Choose a reason for hiding this comment

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

This file can be removed. You still need ./eks-anywhere-baremetal/Testers/Core/snapshot-tester/test-app in the snapshot-tester.yaml inorder to test the test-app. I would rather renname snapshot-tester.yaml to meaningful name and update this Kustomization file.

@mikemcd3912
Copy link
Contributor Author

GH action failed because we're adding/editing a file to a directory w/o a namespace associated with it, which is expected behavior

@mikemcd3912 mikemcd3912 reopened this Mar 13, 2024
Copy link
Contributor

@elamaran11 elamaran11 left a comment

Choose a reason for hiding this comment

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

@mikemcd3912 One comment.

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- claim.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this file removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per our discussion these resource pointer files are unnecessary

Copy link
Contributor

@elamaran11 elamaran11 left a comment

Choose a reason for hiding this comment

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

LGTM

@mikemcd3912 mikemcd3912 merged commit 161f575 into aws-samples:main Mar 13, 2024
1 check failed
@mikemcd3912 mikemcd3912 deleted the bm-tester-snapshot-removal branch May 9, 2024 22:30
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.

2 participants