Skip to content

Activate cloud plugins explicitely #2510

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

Merged
merged 6 commits into from
Aug 29, 2022
Merged

Conversation

rdettai
Copy link
Contributor

@rdettai rdettai commented Aug 18, 2022

Change described in https://app.shortcut.com/tenzir/story/36533/fix-the-way-cloud-plugins-are-loaded

📝 Reviewer Checklist

Review this pull request by ensuring the following items:

  • All user-facing changes have changelog entries
  • User-facing changes are reflected on vast.io

Reviewer instructions

  • Start by checking out the documentation update
  • The main.py contains the new plugin loading logic, so it is a good starting point for the code review itself
  • The core deploy, deploy-step, destroy and destroy-step methods are now fit to deploy/destroy any module, so we removed the module_name.deploy and module_name.destroy commands in modules such as flowlogs and cloudtrail

@rdettai rdettai added enhancement bug Incorrect behavior labels Aug 18, 2022
@rdettai rdettai self-assigned this Aug 18, 2022
@rdettai rdettai marked this pull request as draft August 18, 2022 14:34
@rdettai
Copy link
Contributor Author

rdettai commented Aug 18, 2022

Starts off #2473, so requires it to be merged first

@rdettai
Copy link
Contributor Author

rdettai commented Aug 18, 2022

@rdettai rdettai mentioned this pull request Aug 18, 2022
3 tasks
@rdettai rdettai changed the base branch from master to story/sc-35289/cloud-matchers August 22, 2022 07:16
@rdettai rdettai force-pushed the story/sc-36533/activate-plugins branch from 43772bc to 8e40b50 Compare August 22, 2022 07:22
@rdettai rdettai requested review from dispanser and kaansk August 23, 2022 07:10
@rdettai rdettai marked this pull request as ready for review August 23, 2022 07:10
@rdettai rdettai force-pushed the story/sc-35289/cloud-matchers branch from 9f29769 to 82de3b8 Compare August 24, 2022 08:38
@rdettai rdettai force-pushed the story/sc-36533/activate-plugins branch from cc50c6f to 37fccd7 Compare August 24, 2022 08:44
@rdettai
Copy link
Contributor Author

rdettai commented Aug 24, 2022

@rdettai rdettai force-pushed the story/sc-35289/cloud-matchers branch from 82de3b8 to e6fb8f2 Compare August 25, 2022 06:57
@rdettai rdettai force-pushed the story/sc-36533/activate-plugins branch from 37fccd7 to ab2ff87 Compare August 25, 2022 06:57
Base automatically changed from story/sc-35289/cloud-matchers to master August 25, 2022 07:34
@kaansk
Copy link
Contributor

kaansk commented Aug 25, 2022

Apart from this review, we can discuss about many readme.MD and other documentation files under different folders which makes it really hard to follow. We can just have 1 documentation file including everything.

Additionally, the .env.example template should have following keys as well. These are referenced in aws-pro and can cause confusion.

VAST_PEERED_VPC_ID=
VAST_CIDR=
VAST_AWS_REGION=
VAST_CLOUD_PLUGINS=
VAST_VERSION=
VAST_IMAGE=

Copy link
Contributor

@kaansk kaansk left a comment

Choose a reason for hiding this comment

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

I was able to initialize and destroy pro environment. The issue with environment variables documentation will be followed in another story.

I commented on changelog entry for a typo.

@rdettai rdettai force-pushed the story/sc-36533/activate-plugins branch from ab2ff87 to 27f5fc6 Compare August 29, 2022 07:38
@rdettai
Copy link
Contributor Author

rdettai commented Aug 29, 2022

@kaansk I addressed you request and created a separate internally tracked issue for env variable template.

@rdettai rdettai merged commit f193963 into master Aug 29, 2022
@rdettai rdettai deleted the story/sc-36533/activate-plugins branch August 29, 2022 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants