-
Notifications
You must be signed in to change notification settings - Fork 167
Use existing sample Dockerfiles in CI. Remove beta4 from CI #82
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,18 +6,29 @@ set -e # Exit immediately upon failure | |
| : ${3?"Need to pass TAG as argument"} | ||
| : ${4?"Need to pass VERSION as argument"} | ||
|
|
||
| set +x | ||
|
|
||
| BASE_IMAGE=$1 | ||
| TEST_APP=$2 | ||
| TAG=$3 | ||
| VERSION=$4 | ||
|
|
||
|
|
||
| echo "[CI] Injecting Dockerfile to project $TEST_APP..." | ||
| if [[ ! -d $SAMPLES_REPO/samples/$VERSION/$TEST_APP ]]; then | ||
| echo "[CI] Sample '$TEST_APP' not found for Docker image '$VERSION'" | ||
| exit 1 | ||
| fi | ||
| cd $SAMPLES_REPO/samples/$VERSION/$TEST_APP | ||
| tee Dockerfile << EOF | ||
|
|
||
| ls -al | ||
|
|
||
| if [[ -f "Dockerfile" ]]; then | ||
| echo "Using existing Dockerfile in the sample." | ||
| echo "Dockerfile:" | ||
| cat Dockerfile | ||
| else | ||
| tee Dockerfile << EOF | ||
| FROM $BASE_IMAGE | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: indent?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was on purpose.. |
||
| COPY . /app | ||
| WORKDIR /app | ||
|
|
@@ -26,6 +37,8 @@ ENV DNX_TRACE 1 | |
| ENTRYPOINT sleep 10000 | dnx . kestrel | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since all the samples(including older beta versions with their own respective
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kichalla problem is, if you actually checkout to 1.0.0-beta4 tag for instance, they don't have dockerfiles: https://github.com/aspnet/Home/tree/v1.0.0-beta4/samples/1.0.0-beta4 yet we build them in the CI as well... So keeping this should be making sense as it injects dockerfiles to old versions where it has to be " Maybe it's time for us to retire all those old versions (beta1-2-3-4) today and get going with the newer ones. This could clarify and make the CI scripts a lot precise.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, got it. |
||
| EOF | ||
|
|
||
| fi | ||
|
|
||
| echo "[CI] Building Docker image for $TEST_APP, will tag as '$TAG'..." | ||
| docker build -t $TAG . | ||
| echo "[CI] Built Docker image '$TAG'" | ||
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.
I recall a previous discussion where you mentioned about this script enabling dnx trace...currently the samples in home repo do not enable it (and they should not ideally)...is this a concern?
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.
@kichalla I mean if the kestrel prints the error, the CI collects the container logs as well... So that should be fine.