draios / instruqt-assets

3 stars 3 forks source link

fix init script #10

Closed pmusa closed 2 years ago

pmusa commented 2 years ago

This PR is ready for review. The script was not working due to a few bugs (now fixed). I did not test host and docker yet.

One VERY IMPORTANT thing is that the script should be included in .profile with a bash in front, so it does not exit the entire terminal. For example:

echo "/usr/bin/bash /root/prepare-track/init.sh" >> /root/.profile

The test in the setup script should check for /opt/sysdig/user_data_OK instead of the previous location /usr/local/bin/sysdig/user_data_OK.

Another important thing is that the current test_agent function is not working as it should.

pabloopez commented 2 years ago

Hi Pablo,

I checked (but not tested) this changes and all makes sense, so I won't block this. But, I need to review the previous changes by @Jujuyeh and test them to understand that all is working as we agreed.

This PR is ready for review. The script was not working due to a few bugs (now fixed). I did not test host and docker yet.

I am good with k3s only for now.

One VERY IMPORTANT thing is that the script should be included in .profile with a bash in front, so it does not exit the entire terminal. For example:

echo "/usr/bin/bash /root/prepare-track/init.sh" >> /root/.profile

If these changes are considering this and the configuration part in the track setup script, all good. Right now the only way to persist the configuration status of the track (installing the agent or whatever we are doing in that particular track with init.sh) is to use the .profile config file for the shell. In case the user reloads the lab, or logs out of it or something (but the machine keeps running and the user joins again the session) is to keep this in the logic. This part was working fine before.

The test in the setup script should check for /opt/sysdig/user_data_OK instead of the previous location /usr/local/bin/sysdig/user_data_OK.

Another important thing is that the current test_agent function is not working as it should.

  • it is not testing docker and host.
  • the helm test is confusing and a bit duplicated, it could be cleaned up
  • the timeout is too short
  • the message is weird and does not make much sense

About the new test, the changes that @Jujuyeh proposed make sense. He removed the API query that I implemented (and was not working for different regions) and switched to an approach where the agent logs are greped. This is simpler and more effective.

Why is it not working as it should?