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

add bash demo script #75

Closed

Conversation

nopeppermint
Copy link
Contributor

No description provided.

@nopeppermint nopeppermint changed the title DRAFT: add bash demo script add bash demo script Jan 23, 2025
run_custom_demo.sh Outdated Show resolved Hide resolved
run_custom_demo.sh Outdated Show resolved Hide resolved
run_demo.sh Outdated Show resolved Hide resolved
run_demo.sh Outdated Show resolved Hide resolved
@pellecchialuigi
Copy link
Collaborator

Thanks for addressing my comments!
It looks really good!
May I ask you to mention those scripts in the documentation file?

--network=host \
-v "basil-db-vol:/BASIL-API/db/sqlite3" \
-v "basil-ssh-keys-vol:/BASIL-API/api/ssh_keys" \
-v "basil-tmt-logs-vol:/var/tmp/tmt" \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry I just figured that out...
Volumes need to be used in the api container not in the app.
API is the only project that is able to interact with the DB.

--network=host \
-v "basil-db-vol:/BASIL-API/db/sqlite3" \
-v "basil-ssh-keys-vol:/BASIL-API/api/ssh_keys" \
-v "basil-tmt-logs-vol:/var/tmp/tmt" \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry I just figured that out...
Volumes need to be used in the api container not in the app.
API is the only project that is able to interact with the DB.


podman run \
-d \
--privileged \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

privileged option is not needed for app container
privileged is required for api to be able to spin up containers inside the api container


podman run \
-d \
--privileged \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

privileged option is not needed for app container
privileged is required for api to be able to spin up containers inside the api container

for element in $list;
do
exists="$(podman volume exists "${element}")" # check if volume already exists
if [ "${exists}" ]; then
Copy link
Collaborator

@pellecchialuigi pellecchialuigi Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems that $exists is empty string all the times
I'm trying to run

podman volume exists "${element}"

and then checking the return value

if [ $? == 0]; then
  #exists

@pellecchialuigi
Copy link
Collaborator

I added a script file to deploy the application on a Raspberry Pi as an example of centralized server that anyone can reach in a private network as part of #78
I added some changes to the bash files.
Please have a look and let me know if they make sense to you.
Thanks

@nopeppermint
Copy link
Contributor Author

closing in favor of #78 where you took over this script without giving me credits, not nice!

I tested #78 and it works.

I do not see why you made publicity for rpi, it's working out of the box on my Linux Mint 22.1 as well, so I would expect it to work on any hw with debian based linux without any problem.

Rather than making this rpi specific I would suggest to make it debian specific and and a automated test where you run this script on each commit in CI.

@pellecchialuigi
Copy link
Collaborator

@nopeppermint I don't think there is any reason to close this PR.

I'm sorry, let me clarify.
#78 is addressing a different topic: configuring a centralized server to have an instance of BASIL that can be used in a company/community by multiple users. In that regard I found useful to use a raspberry pi as example as it is a so common and cheap hardware.

I give all the credits for the testing and for the idea of the script to you, really.
We can directly merge this one, I just found that the volumes should apply to api container and not app, and few other suggestions that I tough useful to fix.
Let me reopen this PR, it will be super useful for any new user.

Thanks for testing #78 as well, and the idea of putting it into the CI is really great.
Let me remark here that I really appreciate your contribution.

Moreover I found that some changes are needed to run it on Debian, you can see the details in the PR.

I'll be at FOSDEM next week, presenting BASIL at SBOM track.
if you plan to attend the event let me know, my email is [email protected] we can discuss in person.

@nopeppermint
Copy link
Contributor Author

I am fine with closing this MR as #78 adds everything I was suggesting here, I think #78 is even better as it extends this MR.

I do agree with your code comments.

For next time, I would value a simply "based on MR xyz" in a MR were you take over code or suggesting changes directly in the MR, or merging the MR as is and then extent it.

Let's close this and move forward together ;-)

I'll be at FOSDEM next week, presenting BASIL at SBOM track.

nice, I am not there sorry.

@pellecchialuigi
Copy link
Collaborator

Make sense, I renamed #78 as per your suggestions.
Sure, let's try to step forward with more discussions before merging.
Do you think can be helpful to have a communication channel somewhere?
Anyway #78 is really RPI specific as it is building a separate container file Containerfile-api-rpi that have other needs.
I still see value adding another demo script that is building Containerfile-api, wdyt?

@nopeppermint
Copy link
Contributor Author

I still see value adding another demo script that is building Containerfile-api, wdyt?

not sure, my intent with this MR was to lower the barrier to get BASIL working on my own pc to start evaluating it,
running https://github.com/elisa-tech/BASIL/blob/main/run_demo_rpi.sh does exactly this.

not sure if you really need two different docker files for api, but if then I would suggest name them accordingly, Containerfile-api-fedora and Containerfile-api-debian.

the run_demo_rpi.sh you could rename to run_demo.sh and add an option which api container to start.

I really don't see why rpy is needed there, it's basically a debian docker container.

I would more vote for a comment in the documentation pointing out that there is this run_demo.sh script which allows you to easily setup a working BASIL instance on your local pc, there you can chose if you would like a debian or fedora based image, while the debian based one also tested on a rpy but should work on any debian based linux.

As you have this script now, you might want to change the CI to also use this script and test all it's options automatically.

@pellecchialuigi pellecchialuigi mentioned this pull request Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants