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

Dockerfile cleanup & fix deployment #59

Merged
merged 1 commit into from Jul 2, 2021

Conversation

arnaud-tincelin
Copy link
Collaborator

@arnaud-tincelin arnaud-tincelin commented May 26, 2021

  • cleanup dockerfile & use ENTRYPOINT
  • fix mount paths

@arnaud-tincelin arnaud-tincelin changed the title chore: Dockerfile cleanup Dockerfile cleanup & fix deployment May 27, 2021
@Tatsinnit
Copy link
Member

👍 Looks good, thanks.

I will give it a test from this fork tomorrow, also fyi: @JunSun17 - for name changes and if he can see any other impact.

Copy link
Member

@Tatsinnit Tatsinnit 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, tested with these changes, with following image: tatsat/docker-changes , thanks.

Tool Improvement Plan - Road map automation moved this from In progress to Reviewer approved Jun 30, 2021
Copy link
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

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

💡 THanks for this, just added one small comment as a note as to what we learned few mins back :)

deployment/aks-periscope.yaml Outdated Show resolved Hide resolved
Tool Improvement Plan - Road map automation moved this from Reviewer approved to Review in progress Jun 30, 2021
Copy link
Member

@Tatsinnit Tatsinnit 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, I will give a quick local test and merge it in few hours. Thank you for this.

Tool Improvement Plan - Road map automation moved this from Review in progress to Reviewer approved Jul 1, 2021
Copy link
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

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

💡 Update: Something went wrong and I am still investigating but here is the image created of this branch: tatsat/docker-changes - it constantly cause failure, and logs are empty so something before that went wrong.

Screen Shot 2021-07-02 at 10 46 12 AM

Tool Improvement Plan - Road map automation moved this from Reviewer approved to Review in progress Jul 1, 2021
COPY --from=builder /app/aks-periscope .
CMD ["./aks-periscope"]

COPY --from=builder /build/aks-periscope /
Copy link
Member

@Tatsinnit Tatsinnit Jul 2, 2021

Choose a reason for hiding this comment

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

🐛 I think . lets the bash to identify and run, or might be the entry point below needs a mechanism to tell / to run with bash.

Working image with below change: tatsat/docker-changes-test

Fix \ Solution

COPY --from=builder /build/aks-periscope .

ENTRYPOINT ["./aks-periscope"]

Screen Shot 2021-07-02 at 12 01 09 PM

COPY go.sum .
RUN go mod download

COPY . .
Copy link
Member

Choose a reason for hiding this comment

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

💡 quick question: Just checking for this copy the source and destination is same?

Copy link
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

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

👍 Ok, make sense this is directly associated with the new kustomisation approach and we need to involve az-cli and vscode before we do major release.

If not, we always have option of .. 📓

Thanks @arnaud-tincelin for contribution.

Tool Improvement Plan - Road map automation moved this from Review in progress to Reviewer approved Jul 2, 2021
@Tatsinnit Tatsinnit merged commit 247a539 into Azure:master Jul 2, 2021
Tool Improvement Plan - Road map automation moved this from Reviewer approved to Done Jul 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants