Fix the failed to mv error on python s2i

redhat-developer / odo-init-image

ODO v3 no loner users this image!!! (Container for ODO v2 to setup SupervisorD inside S2I builder image.)

Apache License 2.0

7 stars 27 forks source link

Fix the failed to mv error on python s2i #10

Closed anmolbabu closed 5 years ago

anmolbabu commented 5 years ago

The error happens when the local component source contains a directory by name s2i. This fix clears the $APP_ROOT of all files and folders before the assemble script copies over sources from $destination dir to $app-root so that mv from assemble script does not compile that the folder already exists

fixes https://github.com/redhat-developer/odo/issues/1054 Signed-off-by: anmolbabu anmolbudugutta@gmail.com

anmolbabu commented 5 years ago

This breaks watch :(

cdrage commented 5 years ago

Hey @anmolbabu can you please rebase to master? Tests had broken before.. See #11

anmolbabu commented 5 years ago

Working approach for source deployments :

Use backup-dir(/opt/app-root-src-backup) to backup source in $ODO_S2I_SRC_BIN_PATH..
rm -fr contents of $APP_ROOT/src so that assemble does not complain error in https://github.com/redhat-developer/odo/issues/1054
Trigger assemble script
Copy back source from /opt/app-root-src-backup to $APP_ROOT

This way, the sources from $ODO_S2I_SRC_BIN_PATH are backed up before being moved to $APP_ROOT from which the sources will be deleted to void error in https://github.com/redhat-developer/odo/issues/1054 before the same being movede over from $ODO_S2I_SRC_BIN_PATH(which empties it for the next iteration of watch)

ToDo: Verification of the method being fool proof for every runtime and different combinations of watch and push triggered for binary and local source

cdrage commented 5 years ago

@anmolbabu

It looks like the tests are working as intended (I see that it failed in two sections..)

Make sure you look at @kadel 's PR too as he restores from backup so it may conflict with regards to your PR.

anmolbabu commented 5 years ago

@anmolbabu

It looks like the tests are working as intended (I see that it failed in two sections..)

Make sure you look at @kadel 's PR too as he restores from backup so it may conflict with regards to your PR.

@cdrage Ack Fortunately those lines remained untouched in my PR ;) So hopefully they shouldn't conflict

jorgemoralespou commented 5 years ago

Don't know how to test, but want to make sure this would work:

When doing odo push and it's building, hit CTRL-C and break the execution of odo push. In current release, somehow backup dir is preserved (or saved, don't remember which) and further builds didn't work.

anmolbabu commented 5 years ago

Don't know how to test, but want to make sure this would work:

When doing odo push and it's building, hit CTRL-C and break the execution of odo push. In current release, somehow backup dir is preserved (or saved, don't remember which) and further builds didn't work.

@jorgemoralespou I would say not in the scope of this PR Do you mean hot Ctrl+c should break even an already executing assemble script? odo doesn't intercept SIGINT at the moment I am not sure if there's a way to do it in Go(I have seen it in python though) so may be I can write a new PR for that bcoz I am really not sure how the SIGINT even if caught, can even be communicated to the component pod where the assemble is under execution

Cc: @kadel @cdrage @surajnarwade

kadel commented 5 years ago

@jorgemoralespou I would say not in the scope of this PR

This PR is already a significant change (because it is tight to https://github.com/redhat-developer/odo/pull/1125) Let's fix this right after we are done with this.

Do you mean hot Ctrl+c should break even an already executing assemble script? odo doesn't intercept SIGINT at the moment I am not sure if there's a way to do it in Go(I have seen it in python though) so may be I can write a new PR for that bcoz I am really not sure how the SIGINT even if caught, can even be communicated to the component pod where the assemble is under execution

We need to fix it, this was not an issue before we started doing all this backup and restore charades. Odo should never leave component in a broken state where the only way to fix it is to delete the whole component.

kadel commented 5 years ago

@jorgemoralespou I just compiled odo with https://github.com/redhat-developer/odo/pull/1125 that uses image from this PR to make it easy to test this anywhere.

You can download it here odo.zip ( It is compiled just for MacOS)

anmolbabu commented 5 years ago

@kadel All comments incoporated please review

metacosm commented 5 years ago

I don't feel qualified to review this PR, sorry.

kadel commented 5 years ago

Tests are failing because the new image is not compatible with current odo. (It requires https://github.com/redhat-developer/odo/pull/1125 to work properly)