Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Layer ADD source-code prior to make bootstrap layer in Dockerfile #3767

Merged
merged 1 commit into from Aug 30, 2018

Conversation

ashish-amarnath
Copy link
Contributor

What this PR does / why we need it: Currently, building ACS Engine from Source fails. This PR fixes that.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #3766

Special notes for your reviewer:

If applicable:

  • documentation
  • unit tests
  • tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

Release note:

NONE

@ashish-amarnath
Copy link
Contributor Author

/assign @jackfrancis

@CecileRobertMichon
Copy link
Contributor

/assign @tariq1890
Tariq, this is one of the issues we looked at yesterday

Dockerfile Outdated
@@ -27,7 +27,7 @@ RUN git clone https://github.com/akesterson/shunit.git /tmp/shunit \
WORKDIR /gopath/src/github.com/Azure/acs-engine

# Cache vendor layer
ADD Makefile test.mk versioning.mk Gopkg.toml Gopkg.lock /gopath/src/github.com/Azure/acs-engine/
ADD Makefile test.mk versioning.mk Gopkg.toml Gopkg.lock . /gopath/src/github.com/Azure/acs-engine/
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add packer.mk instead of the period. Adding the entire directory along with specific files seems inconsistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I think it would be an even better idea to add the entire directory, which we do anyway, instead of individual files.
Just adding packer.mk to the list of files logs messages like

fatal: Not a git repository (or any of the parent directories): .git
fatal: Not a git repository (or any of the parent directories): .git
fatal: Not a git repository (or any of the parent directories): .git
fatal: Not a git repository (or any of the parent directories): .git
fatal: Not a git repository (or any of the parent directories): .git

@tariq1890
Copy link
Contributor

@ashish-amarnath Thanks for PR :). Added a review comment.

@codecov
Copy link

codecov bot commented Aug 30, 2018

Codecov Report

Merging #3767 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #3767   +/-   ##
======================================
  Coverage    55.4%   55.4%           
======================================
  Files         108     108           
  Lines       16102   16102           
======================================
  Hits         8921    8921           
  Misses       6417    6417           
  Partials      764     764

@tariq1890
Copy link
Contributor

/lgtm

@acs-bot acs-bot added the lgtm label Aug 30, 2018
@acs-bot
Copy link

acs-bot commented Aug 30, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ashish-amarnath, tariq1890
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: jackfrancis

If they are not already assigned, you can assign the PR to them by writing /assign @jackfrancis in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jackfrancis jackfrancis merged commit 8592e49 into Azure:master Aug 30, 2018
@ashish-amarnath ashish-amarnath deleted the fix-dockerfile-layering branch August 31, 2018 02:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Building ACS Engine from Source fails
5 participants