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

Create script for Quickstart package #534

Closed
ionut-arm opened this issue Sep 28, 2021 · 8 comments · Fixed by #612
Closed

Create script for Quickstart package #534

ionut-arm opened this issue Sep 28, 2021 · 8 comments · Fixed by #612
Labels
documentation enhancement New feature or request small Effort label

Comments

@ionut-arm
Copy link
Member

Official Github releases of the service contain a tar gzip file used for quickstart testing. The contents and steps are described in the Quickstart guide. The process of generating the .tar.gz is currently manual, but a script could be included for easier processing in the future.

If we're confident enough, we could even make use of Github Actions to completely automate it.

@ionut-arm ionut-arm added enhancement New feature or request documentation small Effort label labels Sep 28, 2021
@anta5010
Copy link
Collaborator

parsec-cli-tests.sh can be included into the archive and be mentioned in the Quickstart guide

@mohamedasaker-arm
Copy link
Contributor

@anta5010 I wonder if we can modify the packaged parsec-cli-tests.sh. more precisely this line

PARSEC_SERVICE_ENDPOINT="${PARSEC_SERVICE_ENDPOINT:-unix:/run/parsec/parsec.sock}"

The reason is, that in the release package parsec uses the socket path ./parsec.sock and I think unifying the default socket path would enhance the OOB experience.
Please share your insights

@anta5010
Copy link
Collaborator

@anta5010 I wonder if we can modify the packaged parsec-cli-tests.sh. more precisely this line

PARSEC_SERVICE_ENDPOINT="${PARSEC_SERVICE_ENDPOINT:-unix:/run/parsec/parsec.sock}"

The reason is, that in the release package parsec uses the socket path ./parsec.sock and I think unifying the default socket path would enhance the OOB experience. Please share your insights

Sorry, I don't understand what you mean by "the release package parsec uses the socket path ./parsec.sock".

Parsec service built from sources uses /run/parsec/parsec.sock by default. See

static DEFAULT_SOCKET_PATH: &str = "/run/parsec/parsec.sock";

And the same parameter value is used by default in the default Parsec config file. See

#socket_path = "/run/parsec/parsec.sock"

So, most likely you're using a Parsec config file where socket_path is set to a non-default value for whatever reason. I think instead of changing parsec-cli-tests.sh you can either:

  • fix the config file you're using to use the default value or
  • set PARSEC_SERVICE_ENDPOINT env variable to a non-default value you require. For example PARSEC_SERVICE_ENDPOINT='./parsec.sock' parsec-cli-tests.sh

@mohamedasaker-arm
Copy link
Contributor

@anta5010
yes, indeed as it's mentioned here
It is a non-default path
I was just thinking when someone tries to run the script, then he needs to set PARSEC_SERVICE_ENDPOINT.
Do you think we need to add a section after this for the script ?

@ionut-arm
Copy link
Member Author

You have the freedom to alter the parsec-cli-tests.sh script in this quickstart package script, so you can just do a good ol' search-and-replace, and set the path in the script to whatever you need!

@anta5010
Copy link
Collaborator

@mohamedasaker-arm I still don't understand your issue. Could you explain it for me.
The script behavior is the same as parset-tool. It uses PARSEC_SERVICE_ENDPOINT if defined or lets parsec-tool to use the default path. I don't see any reason to change the script to a non-default value. The scripts is already used in testings which would be broken. Please use PARSEC_SERVICE_ENDPOINT
I also don't quire understand why do you use a non-default value in your environment. Don't we want to have a quickstart sandbox be as close to real-world deployments as possible?

And please-please do not change the script. I don't want to have two different versions of the same scripts in different repos. It will be a nightmare to support and sync.
You're welcome to add instructions for parsec-cli-tests.sh to the quick guide of course.

@mohamedasaker-arm
Copy link
Contributor

Let's first tackle this question
"Why do you use a non-default value in your environment. Don't we want to have a quickstart sandbox be as close to real-world deployments as possible?"
In order to use the default path without securely installing parsec

  • Create or make sure that the /run/parsec path exists
  • Change the owner of the folder

sudo mkdir /run/parsec
sudo chown parsec /run/parsec
sudo chmod 755 /run/parsec

The current quickstart package uses a non-default path to avoid those steps @ionut-arm correct me, please!

Regarding this statement "I don't want to have two different versions of the same scripts in different repos. It will be a nightmare to support and sync."
Practically, the edits won't end up in any repo. However, it's a change and might need to be synced If the env variable name has changed.

I don't think the PARSEC_SERVICE_ENDPOINT will change regularly, but indeed this change will require a sync if it happens.

If the script is packaged as it is, then before using the script user has to set the environment variable PARSEC_SERVICE_ENDPOINT.

@anta5010
Copy link
Collaborator

By changing the script I didn't mean changing the variable. I meant that when we update the script with some new functionality in parsec-tool repo we will have to remember to apply your change when we copy it from parsec-tool to quickstart

Yes, the last paragraph is correct. If you use a non-default environment you just need to include export PARSEC_SERVICE_ENDPOINT="<your path>" once into your instruction.

mohamedasaker-arm added a commit to mohamedasaker-arm/parsec that referenced this issue Jun 7, 2022
Fixes: parallaxsecond#534

Signed-off-by: Mohamed Omar Asaker <mohamed.omarasaker@arm.com>
mohamedasaker-arm added a commit to mohamedasaker-arm/parsec that referenced this issue Jun 7, 2022
Fixes: parallaxsecond#534

Signed-off-by: Mohamed Omar Asaker <mohamed.omarasaker@arm.com>
mohamedasaker-arm added a commit to mohamedasaker-arm/parsec that referenced this issue Jun 8, 2022
Fixes: parallaxsecond#534

Signed-off-by: Mohamed Omar Asaker <mohamed.omarasaker@arm.com>
@mohamedasaker-arm mohamedasaker-arm added this to the Parsec Release 1.1.0 milestone Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation enhancement New feature or request small Effort label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants