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

[addon module] velero - update version and add required values #693

Merged
merged 6 commits into from Jun 30, 2022

Conversation

Hokwang
Copy link
Contributor

@Hokwang Hokwang commented Jun 27, 2022

What does this PR do?

  • user can create bucket in module
  • update velero version to v1.9
  • add required values in values.yaml

Motivation

  • velero v1.9 released
  • some required values not exist

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I have added a new example under examples to support my PR
  • Yes, I have created another PR for add-ons under add-ons repo (if applicable)
  • Yes, I have updated the docs for this feature
  • Yes, I ran pre-commit run -a with this PR

Note: Not all the PRs required examples and docs except a new pattern or add-on added.

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

Copy link
Contributor

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

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

Bucket creation should be left out of the addon/module - there are a number of possible settings for S3 buckets and this is better left up to users to configure rather than the module here

@Hokwang
Copy link
Contributor Author

Hokwang commented Jun 27, 2022

OK, then I will remove bucket creation code.

@Hokwang Hokwang temporarily deployed to EKS Blueprints Test June 27, 2022 13:42 Inactive
@Hokwang Hokwang changed the title [addon module] velero - add bucket creation [addon module] velero - update version and add required values Jun 27, 2022
provider: aws
backupStorageLocation:
bucket: ${bucket}
volumeSnapshotLocation:
Copy link
Contributor

Choose a reason for hiding this comment

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

could you elaborate on some of these changes, why they are important as defaults, etc. please

Copy link
Contributor Author

@Hokwang Hokwang Jun 28, 2022

Choose a reason for hiding this comment

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

Please refer to https://github.com/vmware-tanzu/velero-plugin-for-aws/blob/main/volumesnapshotlocation.md
it is required value.
When you don't use volumeSnapshotLocation.region, install could be success,
but backup always Partialy Failed,
it shows

$ velero backup logs daily-backup3 | grep -v "level=info"
time="2022-06-28T05:19:27Z" level=error msg="Error getting volume snapshotter for volume snapshot location" backup=velero/daily-backup3 error="rpc error: code = Unknown desc = missing region in aws configuration" error.file="/go/src/velero-plugin-for-aws/velero-plugin-for-aws/volume_snapshotter.go:82" error.function="main.(*VolumeSnapshotter).Init" logSource="pkg/backup/item_backupper.go:453" name=pvc-60373ea9-b6cd-4a62-8eb8-d51f3eb0e64c namespace= persistentVolume=pvc-60373ea9-b6cd-4a62-8eb8-d51f3eb0e64c resource=persistentvolumes volumeSnapshotLocation=default

Copy link
Contributor

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

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

CI failure is unrelated to this change - thanks for the update @Hokwang

@bryantbiggs bryantbiggs merged commit 79ef934 into aws-ia:main Jun 30, 2022
@Hokwang Hokwang deleted the patch-2 branch July 1, 2022 15:51
allamand pushed a commit to allamand/terraform-aws-eks-blueprints that referenced this pull request Dec 15, 2022
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.

None yet

3 participants