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.

Sign up for GitHub

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

Jump to bottom

Break out scripts to install and setup #297

Merged
merged 4 commits into from
Aug 12, 2022
Merged

Break out scripts to install and setup #297

merged 4 commits into from
Aug 12, 2022

Conversation

pxp928
Copy link
Member

@pxp928 pxp928 commented Aug 7, 2022

Signed-off-by: pxp928 [email protected]

Breakout the scripts based on issue #292 and the discussion around reorg. Break out scripts to install and setup to allow for ease of install and development.

@pull-request-size pull-request-size bot added the size/L Large (100-499 lines of changes) label Aug 7, 2022
@pxp928 pxp928 mentioned this pull request Aug 7, 2022
Closed
sudo-bmitch
sudo-bmitch reviewed Aug 7, 2022
View reviewed changes
Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

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

Just a quick comment without a detailed look at the other changes. We probably want to document the general stages for file numbering, and whether we can use the same number for different scripts that can be run concurrently.

Makefile Outdated
@@ -33,7 +33,7 @@ setup-minikube: ## Setup a Kubernetes cluster using Minikube
bash platform/00-kubernetes-minikube-setup.sh

.PHONY: setup-frsca
setup-frsca: setup-certs setup-registry setup-tekton-chains setup-spire setup-vault
setup-frsca: setup-certs setup-registry install-tekton-pipelines setup-tekton-pipelines install-tekton-chains setup-tekton-chains install-spire setup-spire install-vault setup-vault
Copy link
Contributor

Choose a reason for hiding this comment

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

Should kyverno be part of frsca? Seems like we'd want it here instead of adding entries to the quickstart.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that it should be added. For some reason we did not have it as part of setup-frsca so I did not move it. Maybe it was a remanent of the old work, but I will add it in.

@pxp928
Copy link
Member Author

pxp928 commented Aug 7, 2022

Just a quick comment without a detailed look at the other changes. We probably want to document the general stages for file numbering, and whether we can use the same number for different scripts that can be run concurrently.

Need to think about this. Need to do some tests to see what can come up concurrently.

@pxp928
Copy link
Member Author

pxp928 commented Aug 7, 2022 •
edited
Loading

Updated setup-frsca to install scripts in parallel as @sudo-bmitch suggested. The file number might need to be captured in a different issue.

sudo-bmitch
sudo-bmitch approved these changes Aug 12, 2022
View reviewed changes
Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

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

LGTM. There's still some stuff I want to change, but I'm adding that to my list to send as a separate PR.

@kodiakhq kodiakhq bot merged commit 84d7a51 into buildsec:main Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sudo-bmitch sudo-bmitch sudo-bmitch approved these changes

@bradbeck bradbeck bradbeck approved these changes

Assignees
No one assigned
Labels
size/L Large (100-499 lines of changes)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pxp928 @bradbeck @sudo-bmitch