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 user/group id config to compose to remove need for sudo #38

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ChrysmOre
Copy link
Contributor

@ChrysmOre ChrysmOre commented Sep 29, 2021

What

Added make-privileged make target and changed regular make start to not use sudo. Removed extraneous scripts and updated references and READMEs to reference make targets and existing scripts instead.

Have also added The Train to the compose journey.

How to test

Test changes make sense.

Test existing helpers and analysis journeys still work (test-docker-compose, start-analysis)

Verify the-train container starts up successfully (will see calls to /v1/health in the logs, albeit they will return 404 as the train doesn't have a health endpoint!)

Copy link
Contributor

@redhug1 redhug1 left a comment

Choose a reason for hiding this comment

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

See direct comments in Slack.
This is not working at all well, if at all on MacBook's

@ChrysmOre ChrysmOre force-pushed the feature/remove-need-for-sudo branch 4 times, most recently from fe7c57b to b1677e1 Compare October 1, 2021 08:42
@ChrysmOre ChrysmOre force-pushed the feature/remove-need-for-sudo branch 2 times, most recently from b44d36f to b7704ab Compare October 11, 2021 10:03
@ChrysmOre ChrysmOre force-pushed the feature/remove-need-for-sudo branch 2 times, most recently from 08db951 to 3524dc1 Compare October 22, 2021 14:48
Copy link
Contributor

@cookel2 cookel2 left a comment

Choose a reason for hiding this comment

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

I just noticed this.


To use the `start-import` helpers scripts or analysis tools you will need
to set an environment variable called `FLORENCE_PASSWORD` to your local
florence login password for `[email protected]`. Alternatively
Copy link
Contributor

Choose a reason for hiding this comment

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

The ons part is missed out:

Suggested change
florence login password for `[email protected]`. Alternatively
florence login password for `florence@magicroundabout.ons.gov.uk`. Alternatively

@ChrysmOre ChrysmOre force-pushed the feature/remove-need-for-sudo branch from 3524dc1 to d1c6a36 Compare October 25, 2021 08:17
@cookel2
Copy link
Contributor

cookel2 commented Oct 26, 2021

In the test-docker-compose section the version of go should probably be changed to 1.17 in go.mod.

Copy link
Contributor

@cookel2 cookel2 left a comment

Choose a reason for hiding this comment

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

If I try to run the start-import.sh then it gives me this error:

Making request to POST import-api/jobs:{"recipe":"38542eb6-d3a6-4cc4-ba3f-32b25f23223a"}
error posting job to importAPI: error performing request: Post "http://localhost:21800/jobs": EOF
exit status 1


* Adjust the constant `maxRuns` in `test-compose.go` for the number of times you want the process to run. Each loop of the process may take about 3 minutes. You may also need to adjust `maxContainersInJob` to match the number of containers that are run for the cantabular import process (which may change).

* First run `./run-cantabular-without-sudo.sh` in its directory and then when all containers running, run `./start-import.sh` in its directory to test that at least one import process completes OK
* First run `make start` and then when all containers running, run `./start-import.sh` in its directory to test that at least one import process completes OK
Copy link
Contributor

Choose a reason for hiding this comment

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

It took me a few minutes to locate the start-import.sh so I think it would be useful to say that it's in the helpers directory.

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.

3 participants