-
Notifications
You must be signed in to change notification settings - Fork 88
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
Entry.sh file for username, password authentication in docker (second phase) #390
Conversation
Entry.sh file for username, password authentication in docker
I have got the following issue attempting to build this branch at dockerhub:
@Jagatjot could you have a look, please? If modifications are needed, please do them as PRs on this PR branch (i.e. using feature/366_pre_merge as base branch). In addition, please ensure that the docker build process works with the Dockerfile (i.e. build it locally so we can detect additional problems that we would got on dockerhub). |
@fgalan - how are you building from the branch? By default the The solution is to tag the hash you want to test and build using the command line syntax found in the Ultralight Twin
|
In fact, I think this is related with the issue I created some time ago telefonicaid/iotagent-ul#317. I forgot about it ;) The problem is that the E2E CI system takes images from dockerhub. So, at the end, I need the image built on dockerhub. Of course I can build locally and push to dockerhub but I'd prefer if possible some sort of solution fully contained in dockerhub. In last comment in the cited issue you said "It looks like there could be a fix soon on the Docker Hub side". Let's check if that "soon" (around 3 months ago) has become reality :) |
Docker Hub and Docker Cloud are now integrated, so it should just be a matter of adding an appropriate build hook and replacing
It would make sense to add things like the githash or build version as |
@fgalan it is because at dockerhub it is searching for docker/entrypoint.sh at master branch and it is not present at master therefore it says : No such file or directory |
@Jagatjot - Try porting over the webhook from the Ultralight twin: |
@@ -3,6 +3,7 @@ FROM node:${NODE_VERSION} | |||
ARG GITHUB_ACCOUNT=telefonicaid | |||
ARG GITHUB_REPOSITORY=iotagent-json | |||
ARG DOWNLOAD=latest | |||
ARG SOURCE_BRANCH=master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the usage of this new ARG be taken into accoun in section "How to build your own image" of the README.md file? What do you think @jason-fox ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - it should now be split into three parts.
see - Ultralight
How to build an image
--- etc
Building from your own fork
To download code from your own fork of the GitHub repository add the arguments to the docker build
command.
docker build -t iot-agent . \
--build-arg GITHUB_ACCOUNT=<your account> \
--build-arg GITHUB_REPOSITORY=<your repo> \
--build-arg SOURCE_BRANCH=<your branch>
Building from your own source files
To download code from your own fork of the GitHub repository add the GITHUB_ACCOUNT
, GITHUB_REPOSITORY
and SOURCE_BRANCH
arguments (default master
) to the docker build
command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in aef4025 (by imitation of jason-fox/iotagent-ul@ac295c0#diff-4b5e6a0d7df359d2d18257ca84179db7fd61ef67)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
@jason-fox I have ported the commits you mention and not it seems it is working. Thanks! So, e2e regression can be done now. Keep tuned :) |
not or now ? I hope it is the latter. |
Related to: telefonicaid/iotagent-node-lib#767 - with the alternative Keyrock authentication, line 46 of entrypoint.sh needs the following: file_env 'IOTA_AUTH_USER'
file_env 'IOTA_AUTH_PASSWORD'
file_env 'IOTA_AUTH_CLIENT_ID'
file_env 'IOTA_AUTH_CLIENT_SECRET' The commented out config section should be amended as well. |
Yes. I was now ;) In fact, the first round seems to be very promissing, but a new round will take place after the changes you have suggested at #390 (comment) On the meanwhile, please have a look to this comment. |
The four added in a35b163 (then fixed in 9dbff11, as some of then already were there).
Add (the two actually new) in f632cab |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is the second phase of the merge for PR #380. The diff +127 -5 is a good checksum ensuring that both PRs has the same content.
E2E CI testing will be done on this PR in order to guarantee nothing is broken. If tests are ok, then it will be merged.
CC: @Jagatjot @jason-fox @dcalvoalonso @AlvaroVega