search

tinkerbell / playground

Example deployments of the Tinkerbell Stack for use as playground environments
Apache License 2.0
131 stars 87 forks source link

Install local tink cli for Vagrant setup #52

Closed stolsma closed 3 years ago

stolsma commented 3 years ago

Description

DO NOT COMMIT!! Still working on it!

Install Tink CLI from Sandbox Github Release location to main Vagrant or Provisioning VM so a docker cli line isn't needed any more.

Why is this needed

Implements #49

Fixes: #49

How Has This Been Tested?

This PR is hand tested as I don't have access to a Vagrant or Terraform installation

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

None, this is just an add-on to the normal Vagrant and Terraform install flow

Checklist:

I have:

stolsma commented 3 years ago

Sorry, didn't sign off correctly. Will correct later... :-(

stolsma commented 3 years ago

A couple of things. I would like to avoid the race condition that the new environment variable referring to the sandbox tag creates.

I don't understand, sorry... there is a Github action (in .github/workflows/tags.yaml) that creates Github release artifacts on tag v* pushes. That version number is invented by someone when tagging. Before tagging that number can be changed in current_versions.sh as in that file also the sha's of the containers need to be changed, that can be done in one sweep when releasing a new version of the Sandbox. Where is my brain taking the wrong turn??

gianarb commented 3 years ago

The workflow you describe is right but you are adding a mandatory extra step (match the tag in the current_version.sh) for no obvious reason. From my perspective, it looks like extra things I have to do as a maintainer. it is not needed, we have the version of the CLI already expressed as a docker image name

gianarb commented 3 years ago

Another thing. We can't bump the version of the CLI until the sandbox is tagged, and that makes testing harder.

On Fri, Feb 5, 2021 at 4:33 PM Gianluca Arbezzano ciao@gianarb.it wrote:

The workflow you describe is right but you are adding a mandatory extra step (match the tag in the current_version.sh) for no obvious reason. From my perspective, it looks like extra things I have to do as a maintainer. it is not needed, we have the version of the CLI already expressed as a docker image name

Raj-Dharwadkar commented 3 years ago

@stolsma , Thanks for your contribution . Please can you update this PR ?

jacobweinstock commented 3 years ago

Hey @stolsma, #90 just landed and has changed the sandbox significantly. Would you mind having a look at the changes and confirm if this change is still something you'd like to see?

stolsma commented 3 years ago

@Raj-Dharwadkar @jacobweinstock sorry for this late reply, was on holiday.

Yep, looking in to #90 it would still be nice to download the cli to another system (could be the vm host system or even another remote one) like what could be done with kubectl. It would make it a lot easier to interact (from the host) with the installed tinkerbell environment on the sandbox vm without having to ssh into the vm and then using docker exec to enter into the tinkerbell-cli container and execute your cli command...

Unfortunately I'm not working on Tinkerbell dev anymore at my current assignment so I will not be able to change this pr such that it will implement my request. Sorry about that... if usable, you may use my pr in a new one if appropriate.

Should I close this pr or will you do it @Raj-Dharwadkar ?

nshalman commented 3 years ago

Should I close this pr or will you do it @Raj-Dharwadkar ?

I can close this out. We can always re-open if necessary. Thanks again!