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

memorystore: add Dockerfile for Cloud Run deployment #1538

Merged
merged 8 commits into from Jul 15, 2020
Merged

memorystore: add Dockerfile for Cloud Run deployment #1538

merged 8 commits into from Jul 15, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 26, 2020

I'm working on a tutorial for connecting to Memorystore from Cloud Run via Serverless VPC Access. The existing sample app still works, we just need a Dockerfile to deploy it. I'm also adding back go.mod and go.sum in the proper location, and including a .gcloudignore.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 26, 2020
@ghost ghost changed the title Add go.mod, go.sum, .gcloudignore along with Dockerfile for Cloud Run… Memorystore: Add Dockerfile for Cloud Run deployment Jun 26, 2020
@tbpg tbpg requested review from grayside and tbpg June 30, 2020 15:08
Copy link
Contributor

@tbpg tbpg left a comment

Choose a reason for hiding this comment

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

I added a couple small commits (go mod tidy and updating the bad file checker).

# Use the official Golang image to create a build artifact.
# This is based on Debian and sets the GOPATH to /go.
# https://hub.docker.com/_/golang
FROM golang:1.13 as builder
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use 1.13 or 1.14? If this is only for Cloud Run, it seems like we should use 1.14?

Copy link
Contributor

Choose a reason for hiding this comment

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

The current "canonical" Dockerfile base image is golang:1.13-buster, though #1541 proposed to update that to 1.14. I'm fine rolling that forward. More significantly, this Dockerfile is a copy of an earlier version of the template used in Cloud Run, it would be best to update this from the Cloud Run Pub/Sub sample Dockerfile

Copy link
Author

Choose a reason for hiding this comment

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

I've updated the Dockerfile based on the Cloud Run Pub/Sub sample, thanks

# Use the official Golang image to create a build artifact.
# This is based on Debian and sets the GOPATH to /go.
# https://hub.docker.com/_/golang
FROM golang:1.13 as builder
Copy link
Contributor

Choose a reason for hiding this comment

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

The current "canonical" Dockerfile base image is golang:1.13-buster, though #1541 proposed to update that to 1.14. I'm fine rolling that forward. More significantly, this Dockerfile is a copy of an earlier version of the template used in Cloud Run, it would be best to update this from the Cloud Run Pub/Sub sample Dockerfile


go 1.13

require github.com/gomodule/redigo v2.0.0+incompatible
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the redigo community has been spinning on what to do about this incompatible flag, possibly 2.0 is meant to be ignored.

Resolution in gomodule/redigo#366 or gomodule/redigo#440 are not clearly documented.

I wonder if it's since been fixed, and re-generating the go.mod and go.sum would help?

Copy link
Author

Choose a reason for hiding this comment

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

Looks like it's fixed - I regenerated go.mod and go.sum

@tbpg tbpg added the automerge Merge the pull request once unit tests and other checks pass. label Jul 14, 2020
@tbpg tbpg changed the title Memorystore: Add Dockerfile for Cloud Run deployment memorystore: add Dockerfile for Cloud Run deployment Jul 14, 2020
@gcf-merge-on-green
Copy link
Contributor

Your PR has attempted to merge for 3 hours. Please check that all required checks have passed, you have an automerge label, and that all your reviewers have approved the PR

@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jul 15, 2020
@tbpg tbpg added kokoro:run Add this label to force Kokoro to re-run the tests. automerge Merge the pull request once unit tests and other checks pass. labels Jul 15, 2020
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 15, 2020
@tbpg tbpg merged commit 8982b07 into GoogleCloudPlatform:master Jul 15, 2020
@tbpg tbpg removed the automerge Merge the pull request once unit tests and other checks pass. label Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants