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

while setting up provisioner, check if osie file path is valid #411

Closed
wants to merge 2 commits into from

Conversation

Cbkhare
Copy link
Contributor

@Cbkhare Cbkhare commented Jan 4, 2021

Description

While creating provisioner, if TB_OSIE_TAR is set then osie file is extracted from the file whose location/absolute path is the value of this variable.

Why is this needed

if TB_OSIE_TAR is set and the file is not present at the given path(value of TB_OSIE_TAR) workflow fails stating the Linux image is not found. Which is valid as osie directory under deploy/state would be empty.

  • check has been added to verify if the file is present in the given path or not.

How Has This Been Tested?

By creating the provisioner.
Provisioner is not created if a tar file is present at the location defined in TB_OSIE_TAR.

on success
Screenshot from 2021-01-04 18-50-51

On failure
Screenshot from 2021-01-04 18-48-44

@Cbkhare Cbkhare requested a review from mmlb January 4, 2021 13:22
@Cbkhare Cbkhare self-assigned this Jan 4, 2021
@Cbkhare Cbkhare added the kind/bug Categorizes issue or PR as related to a bug. label Jan 4, 2021
@codecov
Copy link

codecov bot commented Jan 4, 2021

Codecov Report

Merging #411 (c286026) into master (8ea8a0e) will not change coverage.
The diff coverage is n/a.

❗ Current head c286026 differs from pull request most recent head 1bf3b3d. Consider uploading reports for the commit 1bf3b3d to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #411   +/-   ##
=======================================
  Coverage   32.70%   32.70%           
=======================================
  Files          44       44           
  Lines        3137     3137           
=======================================
  Hits         1026     1026           
  Misses       2019     2019           
  Partials       92       92           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ea8a0e...1bf3b3d. Read the comment docs.

@mmlb
Copy link
Contributor

mmlb commented Jan 6, 2021

Shouldn't this already be caught by tar exiting with an error and set-e being set?

@Cbkhare Cbkhare requested review from gauravgahlot, parauliya and mmlb and removed request for mmlb January 12, 2021 02:51
@Cbkhare
Copy link
Contributor Author

Cbkhare commented Jan 12, 2021

Shouldn't this already be caught by tar exiting with an error and set-e being set?

@mmlb , right, I think the script is not bailing out in that case. With this PR it goes for a graceful exit.

setup.sh Outdated Show resolved Hide resolved
Signed-off-by: Gaurav Gahlot <[email protected]>
@tstromberg
Copy link
Contributor

This PR looks great! The DCO checker noticed that one of your git commits isn't signed properly - can you fix that so that this PR may be merged?

https://github.com/tinkerbell/tink/pull/411/checks?check_run_id=2999422937 has instructions on how to fix this PR.

@gauravgahlot
Copy link
Contributor

gauravgahlot commented Jul 27, 2021

@tstromberg I tried those steps already but didn't work. Will it be okay if I close this PR and create a new one?
I know the original contributor personally and I don't think he is looking into Tinkerbell anymore.

@nshalman
Copy link
Member

@tstromberg I tried those steps already but didn't work. Will it be okay if I close this PR and create a new one?
I know the original contributor personally and I don't think he is looking into Tinkerbell anymore.

@gauravgahlot That would be great. I'll close out this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants