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

Docker compose download link #519

Merged
merged 3 commits into from
Sep 1, 2021

Conversation

displague
Copy link
Member

Description

Rebase of #305 (@MaxPeal) with reviewer requested changes applied.

notably, this variation on the original PR does not include curl --progress.

Why is this needed

Fixes: #306

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

Signed-off-by: MaxPeal <30347730+MaxPeal@users.noreply.github.com>
Signed-off-by: MaxPeal <30347730+MaxPeal@users.noreply.github.com>
@displague displague added the ready-to-merge Signal to Mergify to merge the PR. label Aug 17, 2021
@displague
Copy link
Member Author

displague commented Aug 27, 2021

It looks like mergify is stuck on this validation phase. Is there any way to kick it?

@tstromberg
Copy link
Contributor

@displague - a force push should help.

@tstromberg
Copy link
Contributor

Also, I don't think this fixes #306 - which seemed to be around adding some kind of retry? Am I confused?

@mergify mergify bot merged commit aaf6a8e into tinkerbell:master Sep 1, 2021
@displague displague deleted the docker-compose-download-link branch September 2, 2021 22:10
@displague
Copy link
Member Author

@tstromberg Looks like this closes the issue described in #305 PR body, but not #306 which #305 was overextended to solve.

@jacobweinstock
Copy link
Member

jacobweinstock commented Sep 2, 2021

Hey @displague, do you know where/when setup.sh is used or when it's useful?

@displague
Copy link
Member Author

@jacobweinstock Good question.

This PR was resurrected from another PR from a stale issue resurfaced through triage-party. It seems I may have applied a fix for a file we no longer use.

It looks like setup.sh is no longer part of the directions (at least not in a GH search of this project).
setup.sh (a diverged copy) in the boostrap project is used in the Terraform provisioner.

https://github.com/tinkerbell/sandbox/blob/8550bae883e428ae508c4a6c0174761bdc16e51d/deploy/terraform/main.tf#L65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Signal to Mergify to merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the file pull via curl is not robust and sometimes fail with out error / abort the script
5 participants