🎁 Source embedded hack scripts

[cardil]

Changes

• 🎁 Source embedded hack scripts

/kind enhancement

Fixes #221

[knative-prow]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[cardil]

/test all

[cardil]

This works pretty well, as can be seen in example usage PR: knative-extensions/kn-plugin-event#220. See example build 1568204500513067008.

I have some doubts:

1. The extracted files are not getting removed when the scripts end. On a developer's box, this might be a nuisance. After a while, people would have a large amount of hack scripts extracted to their /tmp directory. I wonder, is that a problem? It's straightforward to clean up - rm -rf /tmp/knative.*.
2. The hack scripts are getting extracted to ARTIFACTS subdirectory. So, the scripts get saved as Prow build artifacts. Maybe better would be to save them to an unrelated temp directory. Or, perhaps we like to save them there, to allow for easier debugging. IDK.
3. I think it would be a good idea to set --verbose flag automatically on Prow build. This could be a follow-up, OFC.

/assign @kvmware
/assign @upodroid
/cc @mgencur

[krsna-m]

I have some doubts:

1. I don't like the idea of lingering files. I think a tmp directory that won't be used normally should be used so that you can rm -rf <dir_name> each pass to ensure that we are working with a clean env.
2. I think that should be a flag. Normally adding to artifacts is not desirable imo, but I see the case of someone wanting to debug and passing a flag to include them.

[cardil]

Re @kvmware:

1. I don't like the idea of lingering files. I think a tmp directory that won't be used normally should be used so that you can rm -rf <dir_name> each pass to ensure that we are working with a clean env.

I don't like untidy programming as well. The problem is that it's hard to know, is it safe already to delete those files. We execute those scripts in all kinds of different ways. We might try adding some trap to remove files at EXIT signal, but it might backfire in some cases.

The other thing is that, most of the runs are leaving report files either way. Personally, I often use export ARTIFACTS=/tmp/knative-junk before doing any development, so everything is contained in a single "junk" folder. That's easy to clean up afterwards.

Also, /tmp should be cleared with reboot, and those hack scripts are only a couple KiB in size. People would need to run literally millions of times, without reboot, to have real impact on a modern laptop drive.

2. I think that should be a flag. Normally adding to artifacts is not desirable imo, but I see the case of someone wanting to debug and passing a flag to include them.

Fair enough. This sounds like a good idea to follow up in separate PR, later.

[krsna-m]

lgtm with followup pr/issue for discussed above.

[upodroid]

lgtm

can you try and do some thing like /tmp/HASH/knative and delete /tmp/HASH/knative after each run ?

[krsna-m]

lgtm

can you try and do some thing like /tmp/HASH/knative and delete /tmp/HASH/knative after each run ?

Something like that would be desirable and would be fulfil the ask I had here: I don't like the idea of lingering files. I think a tmp directory that won't be used normally should be used so that you can rm -rf <dir_name> each pass to ensure that we are working with a clean env.

[cardil]

lgtm

can you try and do some thing like /tmp/HASH/knative and delete /tmp/HASH/knative after each run ?

Let me repeat once again. The problem is that it's hard to know, is it safe already to delete those files. We execute those scripts in all kinds of different ways.

When we delete those files at EXIT, they might be read by the parent script even when we push the scripts to a unique folder each source time.

Also, even if that is not the case currently, people could easily introduce such erroneous behavior in the future. And that would be very hard to figure out.

[cardil]

✋ This PR should be ready for re-review.

I changed:

• The extract directory to be static, so subsequent calls just extract the scripts to the same dir. No cleaning is needed, I think.
• There's automatic verbose mode when running on CI (Prow, Jenkins)
• Tests are using the script extract from the same code (all other repos will use full path: knative.dev/hack/cmd/script)

[krsna-m]

lgtm 👍

[cardil]

I'd like to test this before merging

/hold

[cardil]

The testing PR knative-extensions/kn-plugin-event#307 works well. We can go ahead with merging this.

/hold cancel

[upodroid]

LGTM

Thank you for working on this.

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, upodroid

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [cardil,upodroid]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment