[WIP] Make Docker build script to accept custom base image #373
Conversation
|
|
||
| sudo docker build \ | ||
| --build-arg BASE_IMAGE=$BASE_IMAGE \ | ||
| -t $JETBOT_DOCKER_REMOTE/jetbot:base-$JETBOT_VERSION-$L4T_VERSION \ | ||
| -t jetbot/jetbot:base-$JETBOT_VERSION-$L4T_VERSION \ |
There was a problem hiding this comment.
Is there a reason for removing JETBOT_DOCKER_REMOTE?
There was a problem hiding this comment.
Did not find this is actually being used, so this is just for simplifying things.
There was a problem hiding this comment.
It was used previously for hosting/sharing test containers with different remotes (ie: private ones). I'm not sure if we'll need to do this anymore, but may be easiest / safest to leave it as is until we're sure we don't want to revert back.
| source ../scripts/l4t-version.sh | ||
|
|
||
| if [ -z $BASE_IMAGE ]; then | ||
| if [ $L4T_VERSION = "32.4.4" ]; then |
There was a problem hiding this comment.
Is there a reason to drop 32.4.3 which was previously handled in docker/base/build.sh?
There was a problem hiding this comment.
Also, can we move this logic to configure.sh as an environment variable? This is currently where all other docker build configuration is done.
| @@ -0,0 +1,49 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
I don't see this script used. Can we remove?
There was a problem hiding this comment.
This is tied to the change above to build.sh.
If we don't take that change above, we don't need this.
| then | ||
| export JETBOT_DOCKER_REMOTE=jetbot | ||
| fi | ||
| echo "export L4T_VERSION=$L4T_RELEASE.$L4T_REVISION" |
There was a problem hiding this comment.
Instead of deleting JETBOT_DOCKER_REMOTE. I would recommend hard-setting setting a default value of
export JETBOT_DOCKER_REMOTE=jetbot
This is the smallest change to accomplish what I believe the intent was (to handle L4T versions which are not specified here).
| source ../scripts/l4t-version.sh | ||
|
|
||
| if [ -z $BASE_IMAGE ]; then | ||
| if [ $L4T_VERSION = "32.4.4" ]; then |
There was a problem hiding this comment.
Also, can we move this logic to configure.sh as an environment variable? This is currently where all other docker build configuration is done.
No description provided.