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

use env variable CONTAINER to choose the docker container #644

Merged
merged 10 commits into from
Jun 15, 2023

Conversation

HoxhaEndri
Copy link
Member

@HoxhaEndri HoxhaEndri commented May 30, 2023

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    fixes Docker registry variable for EMBA installation #638

  • What is the current behavior? (You can also link to an open issue here)
    downloads the emba docker

  • What is the new behavior (if this is a feature change)? If possible add a screenshot.
    you can set the env variable CONTAINER to use your container

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

  • Other information:

@HoxhaEndri HoxhaEndri added enhancement New feature or request Installation Installation issues labels May 30, 2023
@HoxhaEndri
Copy link
Member Author

For this to work, first the CONTAINER variable must be exported either in the root environment or executing the installer script mantaining the environment variables of the user: sudo -E ./installer.sh -d.

@m-1-k-3
Copy link
Member

m-1-k-3 commented May 30, 2023

ping @secsonthebeach. Would you like to check this PR and give us your feedback?

@m-1-k-3
Copy link
Member

m-1-k-3 commented Jun 13, 2023

@HoxhaEndri please update the wiki information to document the new installation possibilities

@secsonthebeach
Copy link

secsonthebeach commented Jun 13, 2023

ping @secsonthebeach. Would you like to check this PR and give us your feedback?

Hello, sorry for the delay. I am looking into this. Thank you for picking it up!
Not very smart on Git, is this commit merged to master, or is there some other way to verify a PR?
I pulled master at f2c02ed and ran the installer with the CONTAINER env var set.

The installer was failing at installer/I05... with an error reaching the registry. It appears to still be trying docker.io, rather than our internal proxy/cache

Okay, I've now cloned @HoxhaEndri master and it failed at the same place. It was line 29 of installer/I05... which makes sense because the docker manifest inspect embeddedanalyzer/emba:latest line would need to reference the CONTAINER variable as well.

I've tested that command manually using the same CONTAINER env var and it works (example: private-registry.harbor.domain/dockerhub.reference/embeddedanalyzer/emba)

Could you move the if statement on line 51 of installer/I05... to somewhere else, so that the container URL is set for all other docker calls? Or possibly add it to the arg parser for the overall install script?

With the env var set, changing the docker command on line 29 to docker manifest inspect ${CONTAINER} worked to get past the previous failure.... still running...

installer/I05_emba_docker_image_dl.sh Outdated Show resolved Hide resolved
installer/I05_emba_docker_image_dl.sh Outdated Show resolved Hide resolved
@HoxhaEndri
Copy link
Member Author

@m-1-k-3, on thursday I am going to update the wiki.

@HoxhaEndri HoxhaEndri merged commit f5f9a31 into e-m-b-a:master Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Installation Installation issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker registry variable for EMBA installation
3 participants