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

Add cloudbuild.yaml deployments for ePoxy container on GCE #76

Merged
merged 15 commits into from
Feb 25, 2019

Conversation

stephen-soltesz
Copy link
Contributor

@stephen-soltesz stephen-soltesz commented Feb 22, 2019

This change adds a new deployment method for the ePoxy server.

The epoxy boot server is still built into a docker container, but now that container is run from a GCE VM., using Cloud Builder. This change facilitates our usage of LetsEncrypt and private CA certificates.

The deployment creates a new VM, verifies that it is running, and then swaps the static IP from the current epoxy VM to the new one to complete the deployment.


This change is Reviewable

@coveralls
Copy link

coveralls commented Feb 22, 2019

Coverage Status

Coverage remained the same at 89.73% when pulling 34cc570 on sandbox-soltesz-cb into 68fc618 on dev.

Copy link
Contributor

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

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

A couple of small formatting questions, and one question about cluster regions.

Reviewed 3 of 5 files at r1.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @nkinkade and @stephen-soltesz)


cloudbuild.yaml, line 25 at r1 (raw file):

   # NOTE: Changing zones will require manual intervention.
   - 'ZONE_mlab_sandbox=us-east1-c'
   - 'ZONE_mlab_staging=us-central1-b'

Is now a good time to reconsider our region strategy? At one point all three projects were in different regions. Then we decided that perhaps it would just be easier to have them all in the same region. Then at some point we started picking the region based on where ePoxy happened to be deployed in AppEngine.

Do we want all projects in the same region? Or go back to having them all in different regions?


deploy_epoxy_container.sh, line 29 at r1 (raw file):

if [[ -z "${PROJECT}" || -z "${CONTAINER}" || -z "${ZONE}" ]] ; then
    echo "ERROR: Failed to lookup GCP ZONE ('$ZONE') for project '$PROJECT'"

This error message isn't fully indicative of what might have caused it.


deploy_epoxy_container.sh, line 37 at r1 (raw file):

        --format "value(address)" --region "${ZONE%-*}" epoxy-boot-api )
if [[ -z "${IP}" ]] ; then
    echo "ERROR: Failed to find static IP in region ${ZONE%-*}"

Format question: I seem to recollect that we decided to use Google's shell script style guide, which specifies a 2 space indention, right?


deploy_epoxy_container.sh, line 43 at r1 (raw file):

# Lookup the instance (if any) currently using the static IP address for ePoxy.
gce_url=$( gcloud compute addresses describe --project "${PROJECT}" \
		     --format "value(users)" --region "${ZONE%-*}" epoxy-boot-api )

Tabs?


deploy_epoxy_container.sh, line 77 at r1 (raw file):

    --zone "${ZONE}" \
	--tags allow-epoxy-ports \
	--scopes default,datastore \

More tabs.


deploy_epoxy_container.sh, line 112 at r1 (raw file):

    --project "${PROJECT}" \
    --access-config-name "external-nat" --address "$IP" \
	"${UPDATED_INSTANCE}"

Tab.

Copy link
Contributor Author

@stephen-soltesz stephen-soltesz left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @nkinkade)


cloudbuild.yaml, line 25 at r1 (raw file):

Previously, nkinkade wrote…

Is now a good time to reconsider our region strategy? At one point all three projects were in different regions. Then we decided that perhaps it would just be easier to have them all in the same region. Then at some point we started picking the region based on where ePoxy happened to be deployed in AppEngine.

Do we want all projects in the same region? Or go back to having them all in different regions?

Good question. With the migration of the epoxy server to GCE we don't have the dependency on App engine region any more. So, this is possible. Matt talking about zone PCRs last week made me nervous again about putting all our eggs in one basket. But, maybe different zones in the same region could be okay.


deploy_epoxy_container.sh, line 29 at r1 (raw file):

Previously, nkinkade wrote…

This error message isn't fully indicative of what might have caused it.

Done.


deploy_epoxy_container.sh, line 37 at r1 (raw file):

Previously, nkinkade wrote…

Format question: I seem to recollect that we decided to use Google's shell script style guide, which specifies a 2 space indention, right?

I've applied - https://github.com/tmknom/shfmt using the "Google's Shell Style Guide" options.

I'd rather adopt that tool as a convention rather than worry about it.


deploy_epoxy_container.sh, line 43 at r1 (raw file):

Previously, nkinkade wrote…

Tabs?

Done.


deploy_epoxy_container.sh, line 77 at r1 (raw file):

Previously, nkinkade wrote…

More tabs.

Done.


deploy_epoxy_container.sh, line 112 at r1 (raw file):

Previously, nkinkade wrote…

Tab.

Done.

Copy link
Contributor

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 5 files at r1, 1 of 1 files at r2.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @stephen-soltesz)


cloudbuild.yaml, line 25 at r1 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…

Good question. With the migration of the epoxy server to GCE we don't have the dependency on App engine region any more. So, this is possible. Matt talking about zone PCRs last week made me nervous again about putting all our eggs in one basket. But, maybe different zones in the same region could be okay.

If these zones will somehow end up baked into the Mellanox ROM images, then I think we should make up our minds about this now, otherwise I guess can just defer the conversation.

Copy link
Contributor Author

@stephen-soltesz stephen-soltesz left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @nkinkade)


cloudbuild.yaml, line 25 at r1 (raw file):

Previously, nkinkade wrote…

If these zones will somehow end up baked into the Mellanox ROM images, then I think we should make up our minds about this now, otherwise I guess can just defer the conversation.

They do not. The only fixed elements of the ROM are the epoxy server hostname and the root CAs that it will trust.

@stephen-soltesz stephen-soltesz merged commit 4542c15 into dev Feb 25, 2019
@stephen-soltesz stephen-soltesz deleted the sandbox-soltesz-cb branch August 12, 2022 17:03
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.

3 participants