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

Added curl command to docs to pull Karpenter cloud formation template #470

Merged
merged 9 commits into from
Jun 23, 2021
Merged

Added curl command to docs to pull Karpenter cloud formation template #470

merged 9 commits into from
Jun 23, 2021

Conversation

njtran
Copy link
Contributor

@njtran njtran commented Jun 22, 2021

Same PR as #420.

Description of changes:
Cutting this PR since I can't resolve the merge conflicts on the OP's branch.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

bwagner5
bwagner5 previously approved these changes Jun 22, 2021
Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

Lgtm

aws cloudformation deploy \
TEMPOUT=$(mktemp)
curl -fsSL https://raw.githubusercontent.com/awslabs/karpenter/v0.2.6/docs/aws/karpenter.cloudformation.yaml > $TEMPOUT \
| aws cloudformation deploy \
Copy link
Contributor

Choose a reason for hiding this comment

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

For my understanding, why do we need to pipe from curl to aws cli when we are passing this file as a --template-file ${TEMPOUT}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that there's no difference here. If anything piping softly enforces that they run this command together rather than separate, which is the intended flow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to run them together as same command?
If the first command fails its still going to try and execute the aws cloudformation deploy and specifically pipe is confusing because to me it seems like we are passing some output from first command as input to another command.

We could do something like this -

curl -fsSL https://raw.githubusercontent.com/awslabs/karpenter/v0.2.6/docs/aws/karpenter.cloudformation.yaml > $TEMPOUT && aws cloudformation deploy ...

This way if curl command fails we will not be executing the aws cli.

Copy link
Contributor

@prateekgogia prateekgogia left a comment

Choose a reason for hiding this comment

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

LGTM!

@njtran njtran merged commit 5fdb59e into aws:main Jun 23, 2021
@njtran njtran deleted the curlDocs branch July 1, 2021 19:54
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.

4 participants