EQS 81 - Create Proxy Service Infrastructure #21
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| steps: | ||
| # Builds the Docker image | ||
| - name: docker | ||
| id: build_proxy_service_image | ||
| entrypoint: "sh" | ||
| args: | ||
| - "-c" | ||
| - | | ||
| echo "*************************************************************" | ||
| echo "* Building Docker image for environment... *" | ||
| echo "*************************************************************" | ||
| docker build -t "$_EQ_CIR_PROXY_SERVICE_IMAGE_REPO:${SHORT_SHA}" . | ||
|
|
||
| # Pushes the Docker image to Artifact Registry | ||
| - name: docker | ||
| id: push_proxy_service_image | ||
| entrypoint: "sh" | ||
| waitFor: | ||
|
Contributor
There was a problem hiding this comment. These steps are all run in sequence, so don't think we'd need |
||
| - build_proxy_service_image | ||
| args: | ||
| - "-c" | ||
| - | | ||
| echo "*************************************************************" | ||
| echo "* Pushing Docker image to Artifact Registry... *" | ||
| echo "*************************************************************" | ||
| docker push "$_EQ_CIR_PROXY_SERVICE_IMAGE_REPO:${SHORT_SHA}" | ||
|
|
||
| # Deploys the Docker image to Cloud Run | ||
| - name: "gcr.io/google.com/cloudsdktool/cloud-sdk:alpine" | ||
| id: deploy_proxy_service | ||
| entrypoint: "sh" | ||
| args: | ||
| - "-c" | ||
| - | | ||
| echo "*************************************************************" | ||
| echo "* Deploying Docker image to Cloud Run... *" | ||
| echo "*************************************************************" | ||
| gcloud run deploy cir-proxy-service \ | ||
|
Contributor
There was a problem hiding this comment. I think this might lose all of the vars we're passing in for our Terraform deployment? E.g. |
||
| --image "$_EQ_CIR_PROXY_SERVICE_IMAGE_REPO:$SHORT_SHA" \ | ||
| --region europe-west2 \ | ||
| --platform managed | ||
| timeout: 1800s | ||
| options: | ||
| logging: CLOUD_LOGGING_ONLY | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| steps: | ||
|
Contributor
There was a problem hiding this comment. I might be misunderstanding, but I'm not certain what this file is to be used for, as it's not used in the PR linked to this? I'm guessing that this file might be for our formal deployments, as I can see that it's using Also, I'm not sure if this will preserve the same flags that we use for the Cloud Run deployment config in the infrastructure? There was a problem hiding this comment. Yes, this would be for the deployment to prod or anywhere deploy only is required. We are looking to name the scripts after the function they carry out. So maybe cloudbuld-deploy-only.yaml would be an better name.
Contributor
There was a problem hiding this comment. As there is currently no reference to this file in our infrastructure repo, does that mean we will be manually creating a deployment trigger in our formal environments which uses this file, outside of the Terraform altogether? If that's the case, we might want to document this somewhere as part of our run books, release strategy documentation, or maybe as a comment in the file? |
||
| # Deploys the Docker image with the specified tag to Cloud Run | ||
| - name: "gcr.io/google.com/cloudsdktool/cloud-sdk:alpine" | ||
| id: deploy_proxy_service | ||
| entrypoint: "sh" | ||
| args: | ||
| - "-c" | ||
| - | | ||
| gcloud run deploy cir-proxy-service \ | ||
|
Contributor
There was a problem hiding this comment. I think this might lose all of the vars we're passing in for our Terraform deployment? E.g. LOG_LEVEL isn't being passed in here |
||
| --image "$_EQ_CIR_PROXY_SERVICE_IMAGE_REPO:$TAG_NAME" \ | ||
| --region europe-west2 \ | ||
| --platform managed | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| steps: | ||
|
Contributor
There was a problem hiding this comment. If we're going to lint and test in Cloud Build (instead of GitHub Actions), I think we might want to also update this for the Converter Service too? Also, we're still using GitHub Actions for linting and testing as it stands too? There was a problem hiding this comment. Yes, we can do that for convertor too. We did discuss getting this through and review the best approach in a later ticket. At that discussion we can decide if we want to get rid of github actions.
Contributor
There was a problem hiding this comment. Okay no worries. Just a quick follow up - if we're running linting and testing both in GitHub Actions and in GCP, then it might be worth a temporary comment just to call this out? Otherwise it may be a bit confusing for others if we're intentionally doing both in the interim? There was a problem hiding this comment. Had a couple of thoughts about this since it came up during the session, and the pros and cons of running lint/test in cloud build vs actions. From a developer QoL perspective, is it not easier to investigate failing tests within the Pull Request within GitHub rather than having to context switch to GCP and navigating the cloudbuild logs? Could just be me but would be curious to see the opinions of the other devs. On top of this as commented below, it opens up some issues such as needing to run docker within docker for things like Megalinter, which I believe can potentially be a security vulnerability, and is certainly not best practice. I think we also need to ensure when running megalinter in cloudbuild that it is making use of the same config as when it is running locally. There was a problem hiding this comment. RE the first point, we've had tests in cloud build for a long time now and noone has had issue with switching into GCP to see the test results. For me having all the CI scripts in one place and format, rather than mix of both is preferable. There was a problem hiding this comment. I'm happy for the script to be rejigged to remove the SH entry point and to use the default entry point |
||
| # Runs Megalinter on the codebase | ||
| - name: docker | ||
| id: megalint_proxy_service | ||
| entrypoint: "sh" | ||
| args: | ||
| - "-c" | ||
| - | | ||
| echo "*************************************************************" | ||
| echo "* Running Megalinter on the codebase... *" | ||
| echo "*************************************************************" | ||
| docker run --platform linux/amd64 --rm \ | ||
|
Contributor
There was a problem hiding this comment. I think this build There was a problem hiding this comment. I does look odd, will need to break down what this is doing exactly
Contributor
There was a problem hiding this comment. Would this be done as a separate piece of work? There was a problem hiding this comment. Happy for it to be changed to argument based approach with the default entry point, rather than the sh entry point |
||
| -v /var/run/docker.sock:/var/run/docker.sock:rw \ | ||
| -v $(pwd):/tmp/lint:rw \ | ||
| oxsecurity/megalinter-python:v8.8.0 | ||
|
|
||
| # Tests the codebase | ||
| - name: python:3.12.6 | ||
| id: test_proxy_service | ||
| entrypoint: "sh" | ||
| args: | ||
| - "-c" | ||
| - | | ||
| pip install --upgrade pip | ||
| pip install poetry==2.1.2 | ||
| make install-dev | ||
| echo "*************************************************************" | ||
| echo "* Testing the codebase... *" | ||
| echo "*************************************************************" | ||
| make test | ||
|
|
||
| timeout: 1800s | ||
| options: | ||
| logging: CLOUD_LOGGING_ONLY | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| steps: | ||
| # Tags the Docker image with the release tag | ||
| - name: "gcr.io/google.com/cloudsdktool/cloud-sdk:alpine" | ||
| id: tag_proxy_service_release | ||
| entrypoint: /bin/bash | ||
| args: | ||
| - "-c" | ||
| - | | ||
| gcloud container images add-tag \ | ||
| $_EQ_CIR_PROXY_SERVICE_IMAGE_REPO:$SHORT_SHA \ | ||
|
Contributor
There was a problem hiding this comment. I might be misunderstanding, but for continuous delivery for our staging env, I would have thought we'd want to tag with There was a problem hiding this comment. This script is tag and deploy. it would be used for release to preprod after a release tag has been set in github
Contributor
There was a problem hiding this comment. As there is currently no reference to this file in our infrastructure repo, does that mean we will be manually creating a deployment trigger in our formal environments which uses this file, outside of the Terraform altogether? If that's the case, we might want to document this somewhere as part of our run books, release strategy documentation, or maybe as a comment in the file? |
||
| $_EQ_CIR_PROXY_SERVICE_IMAGE_REPO:$TAG_NAME | ||
|
|
||
| # Deploys the Docker image to Cloud Run | ||
| - name: "gcr.io/google.com/cloudsdktool/cloud-sdk:alpine" | ||
| id: deploy_proxy_service | ||
| entrypoint: "sh" | ||
| args: | ||
| - "-c" | ||
| - | | ||
| gcloud run deploy cir-proxy-service \ | ||
| --image "$_EQ_CIR_PROXY_SERVICE_IMAGE_REPO:$TAG_NAME" \ | ||
| --region europe-west2 \ | ||
| --platform managed | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite different to the one we have in the converter service: https://github.com/ONSdigital/eq-cir-converter-service/blob/main/cloud-build-on-merge-dev.yaml, and it seems to be named differently? I think the Converter Cloud Build file might have the advantage of being more concise, not relying on shell script for the steps (so might be less error prone), and matches Cloud Build best practices? We should aim for consistency across repos I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are based of the authors cloudbuild scripts, which have been about for a while and used to be how it was done. We can update this to replace "docker" with "gcr.io/cloud-builders/docker", using the shell entry point does allow us to add log message, but i'm happy to drop those and use the default entry point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if using
SHORT_SHAwould work as expected for our staging env. In the Terraform, we're setting thelatesttag in our staging environment tfvars to allow for continuous delivery/deployment. If we deploy with this particular file, then we would also need to keep updating our tfvars with theSHORT_SHAvar each time we want to make a deployment. Usinglatesttag for this would prevent this I think?