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

Patch quickstart workflow for devcontainer #259

Merged
merged 25 commits into from
Oct 14, 2019

Conversation

clmccart
Copy link
Contributor

@clmccart clmccart commented Oct 1, 2019

What this PR does / why we need it:
This PR patches a few small issues with the devcontainer quickstart workflow.

  • Injects environmeent variables from .env file into .devcontainer/Dockerfile and ./Dockerfile so that user does not have to pass secrets in multiple locations.
  • Installs sudo in the devcontainer (needed for downstream mv commands)
  • runs "make generate" before build command in the Dockerfile

Special notes for your reviewer:
To Test:

  • spin up a new devcontainer by cloning the repo, adding the .env file following README quickstart guidelines. Devcontainer and solution should build with no errors.

If applicable:

this PR contains documentation
this PR contains tests

@frodopwns
Copy link
Contributor

@Azadehkhojandi could you have someone on your side who is familiar with the devcontainer workflow take a look at this PR?

replace hard coded http status code (#255)
Copy link
Contributor

@jananivMS jananivMS left a comment

Choose a reason for hiding this comment

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

Looks good to me and I've tested it and it works too. But will wait for Stephen, Az, Priya to also take a look before approving.

@jananivMS jananivMS removed the request for review from szoio October 7, 2019 21:16
Copy link
Contributor

@JasonTheDeveloper JasonTheDeveloper left a comment

Choose a reason for hiding this comment

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

There's a potential issue if new users try to build the Dockerfile if zz_generated.deepcopy.go isn't generated prior. You've added RUN make generate to the Dockerfile but because hack has been added to .dockerignore, zz_generated.deepcopy.go isn't generated inside the container and if make generate isn't executed prior to building the image, zz_generated.deepcopy.go wont be copied into the image, and as a result the image will fail to build.

I would suggest running make generate when running make set-kindcluster and removing RUN make generate from the Dockerfile, or remove hack from .dockerignore to allow boilerplate.go.txt to be copied into the image.

Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@JasonTheDeveloper JasonTheDeveloper left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@clmccart clmccart merged commit 92da9a1 into Azure:master Oct 14, 2019
@clmccart clmccart deleted the patch/devcont_quickstart branch October 14, 2019 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants