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

Decouple release manifests #716

Closed
wants to merge 4 commits into from

Conversation

bourgeoisor
Copy link
Member

@bourgeoisor bourgeoisor commented Feb 10, 2022

This PR simplifies the release making process and increases the consistency with the Bank of Anthos process (https://github.com/GoogleCloudPlatform/bank-of-anthos/blob/main/release/make-release.sh)

Changes to note:

  • the make-release.sh script(s) has had its lines-of-code reduced by 120 (from 190 to 70)
  • the kubernetes-manifests/ directory was renamed to dev-kubernetes-manifests/ (the "dev" manifests)
  • the release/ directory was renamed to kubernetes-manifests/ (the "release" manifests)
  • the istio-manifests/ directory stays as-is

TODOs:

  • Updated docs
  • Update .yaml files of GitHub workflows
  • Reinstate the region tags

Open questions:

Testing

I have tested this with a fake release (v1.1.1):

~ ./hack/make-release.sh

+ REPO_PREFIX=gcr.io/<project-id>/msd
+ REPO_PREFIX_SED='gcr.io\/<project-id>\/msd'
+++ realpath -s ./hack/make-release.sh
++ dirname /Users/obourgeois/workspace/repos/microservices-demo/hack/make-release.sh
+ SCRIPT_DIR=/Users/obourgeois/workspace/repos/microservices-demo/hack
+ REPO_ROOT=/Users/obourgeois/workspace/repos/microservices-demo/hack/..
+ cd /Users/obourgeois/workspace/repos/microservices-demo/hack/..
+ [[ ! v1.1.1 =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]
+ command -v gcloud
+ rm -rf /Users/obourgeois/workspace/repos/microservices-demo/hack/../kubernetes-manifests
+ mkdir /Users/obourgeois/workspace/repos/microservices-demo/hack/../kubernetes-manifests
+ cp -a /Users/obourgeois/workspace/repos/microservices-demo/hack/../dev-kubernetes-manifests/. /Users/obourgeois/workspace/repos/microservices-demo/hack/../kubernetes-manifests/
+ find /Users/obourgeois/workspace/repos/microservices-demo/hack/../kubernetes-manifests -name '*.yaml' -exec sed -i -e 's'\''image: \(.*\)'\''image: gcr.io/<project-id>/msd\/\1:v1.1.1'\''g' '{}' ';'
+ skaffold config set local-cluster false
set value local-cluster to false for context gke_<project-id>_us-central1-c_boa-tests
+ skaffold build --default-repo=gcr.io/<project-id>/msd --tag=v1.1.1
Generating tags...
 - emailservice -> gcr.io/<project-id>/msd/emailservice:v1.1.1
 - productcatalogservice -> gcr.io/<project-id>/msd/productcatalogservice:v1.1.1
 - recommendationservice -> gcr.io/<project-id>/msd/recommendationservice:v1.1.1
 - shippingservice -> gcr.io/<project-id>/msd/shippingservice:v1.1.1
 - checkoutservice -> gcr.io/<project-id>/msd/checkoutservice:v1.1.1
 - paymentservice -> gcr.io/<project-id>/msd/paymentservice:v1.1.1
 - currencyservice -> gcr.io/<project-id>/msd/currencyservice:v1.1.1
 - cartservice -> gcr.io/<project-id>/msd/cartservice:v1.1.1
 - frontend -> gcr.io/<project-id>/msd/frontend:v1.1.1
 - adservice -> gcr.io/<project-id>/msd/adservice:v1.1.1
 - loadgenerator -> gcr.io/<project-id>/msd/loadgenerator:v1.1.1

The images were pushed on my registry:

image

image

@bourgeoisor bourgeoisor requested a review from a team as a code owner February 10, 2022 19:03
@snippet-bot
Copy link

snippet-bot bot commented Feb 10, 2022

Here is the summary of changes.

You are about to delete 2 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@bourgeoisor bourgeoisor changed the title Issue 375 decouple release manifests Decouple release manifests Feb 10, 2022
@NimJay
Copy link
Collaborator

NimJay commented Feb 10, 2022

Thanks for initiating this, @bourgeoisor!
This PR is definitely going to take us some time.

I assume the changes from the PR will definitely require a "major" bump in the versioning of Online Boutique.
So the release after merging this PR would be v1.0.0 — not v0.3.x.
Reason: kubectl apply -f ./release/kubernetes-manifests.yaml will no longer work — which is used by a lot of docs/videos/README.md/etc.
Do you think we should create a separate v0.3.7 release first — since a lot has changed on main since v0.3.6?

@bourgeoisor bourgeoisor added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 13, 2022
@NimJay
Copy link
Collaborator

NimJay commented Mar 25, 2022

Note from 1-on-1 with Olivier:
We can keep the release/ folder (until all tutorials/docs are updated).

@NimJay NimJay marked this pull request as draft July 25, 2022 13:25
@mathieu-benoit
Copy link
Contributor

Out of curiosity, @NimJay and @bourgeoisor, is this PR still a thing? Or should we close it and reconsider this in a future/following PR? We have limited number of PR opened with Renovate, so wondering if we can close this one?

@NimJay
Copy link
Collaborator

NimJay commented Aug 31, 2022

I'm happy with closing it (but also fine with leaving it open, especially, if we expect to make progress on this in the coming weeks).
I'll leave it up to @bourgeoisor to decide. :)

@bourgeoisor
Copy link
Member Author

Let's close for now -- We can revisit when we implement kustomize and co.

@NimJay NimJay deleted the issue-375-decouple-release-manifests branch November 7, 2022 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Indicates a pull request not ready for merge, due to either quality or timing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants