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

Release/v0.6.0 #1425

Merged
merged 18 commits into from
Apr 24, 2023
Merged

Release/v0.6.0 #1425

merged 18 commits into from
Apr 24, 2023

Conversation

bourgeoisor
Copy link
Member

@bourgeoisor bourgeoisor commented Apr 6, 2023

This PR releases Bank of Anthos v0.6.0.

This is the first release since the refactoring of the CI/CD pipeline to support Kustomize, Cloud Build, Cloud Deploy, and Artifact Registry. As such, it warrants at minimum a bump of the Y component of SemVer.

Draft release notes: https://github.com/GoogleCloudPlatform/bank-of-anthos/releases/edit/untagged-b4569a5ccb5058050ab7

Open questions

About the releasing directory

Edit: I've decided to keep the releasing directory as-is (that is, kubernetes-manifests/) for now. We can revisit this story later.

Historically and still in this PR, the release manifests are stored in the kubernetes-manifests/ directory. Given that these are not the original / source of truth manifests (those live in src/ alongside their relevant services) and especially now that they are explicitly rendered (using skaffold render), I wonder if perhaps "kubernetes-manifests" is a misleading name. It may make sense to try to align ourselves to Online Boutique who uses a release/ directory for release manifests (see here). Of course, this would mean keeping the kubernetes-manifests/ for a little while longer (essentially having a duplicate) while various tutorials (+ this repo's READMEs and tutorials) get fixed up to the newer directory. That said, if there was ever a time to do that change, this would be it (at the same time as all the CI/CD and rendering changes).

The following may also need to be tweaked / fixed:

About the releasing manifests

Edit: I've decided to manually remove the duplicate k8s entries in for now. I have created a tracking issue: #1437

Previously, each services had their own rendered manifest files (alongside the two DB services, the JWT key, and the various ConfigMaps). With the current skaffold render setup, a few files goes away:

  • accounts-db.yaml gets duplicated inside of the contacts and userservice files
  • ledger-db.yaml gets duplicated inside of the balancereader, ledgerwiter, and transactionhistory files
  • jwt-secret.yaml and config.yaml both get duplicated inside of every services

This is intentionally how the Sakffold renderer should work (if we ask it to render a specific service, it will also render out its direct dependencies) and does still run as long as we make sure that no tutorial or other content gets broken because of it (what if a tutorial is explicitly trying to run kubectl apply -f config.yaml for example?) but it does look messy when applying Bank of Anthos on a cluster, getting a handful of unchanged from duplicate objects:

. . .
secret/jwt-key unchanged
deployment.apps/loadgenerator created
configmap/ledger-db-config unchanged
service/ledger-db unchanged
statefulset.apps/ledger-db unchanged
namespace/bank-of-anthos-development unchanged
serviceaccount/bank-of-anthos unchanged
configmap/demo-data-config unchanged
configmap/environment-config unchanged
configmap/service-api-config unchanged
secret/jwt-key unchanged
service/transactionhistory created
deployment.apps/transactionhistory created
namespace/bank-of-anthos-development unchanged
serviceaccount/bank-of-anthos unchanged
configmap/demo-data-config unchanged
. . .

I wonder if it would make sense to try to mimic the previous behaviour as much as possible (no duplicates anywhere, DBs in their own files, and ConfigMaps in their own file). This may unfortunately require manual scripting, since it goes beyond what the Skaffold renderer tries to do. Alternatively, we could render everything in a single file (no duplicate would get generated) but this would mean users couldn't easily pick-and-choose services (and would definitely break some tutorials).

Testing

I have deployed on a default 3-node GKE Standard cluster:

~ kubectl apply -f ./extras/jwt/jwt-secret.yaml
secret/jwt-key created

~ kubectl apply -f ./kubernetes-manifests
configmap/accounts-db-config created
service/accounts-db created
statefulset.apps/accounts-db created
service/balancereader created
deployment.apps/balancereader created
configmap/environment-config created
configmap/service-api-config created
configmap/demo-data-config created
serviceaccount/bank-of-anthos created
service/contacts created
deployment.apps/contacts created
service/frontend created
deployment.apps/frontend created
configmap/ledger-db-config created
service/ledger-db created
statefulset.apps/ledger-db created
service/ledgerwriter created
deployment.apps/ledgerwriter created
deployment.apps/loadgenerator created
service/transactionhistory created
deployment.apps/transactionhistory created
service/userservice created
deployment.apps/userservice created

~ kubectl get pods
NAME                                 READY   STATUS    RESTARTS   AGE
accounts-db-0                        1/1     Running   0          14m
balancereader-69f87fcf6-qpwxl        1/1     Running   0          14m
contacts-557b674495-pxjj8            1/1     Running   0          14m
frontend-64c79c9c57-9zqg4            1/1     Running   0          14m
ledger-db-0                          1/1     Running   0          14m
ledgerwriter-65d4d5b5dc-7l4vs        1/1     Running   0          14m
loadgenerator-5bcb85f657-8v2p8       1/1     Running   0          14m
transactionhistory-8fbc64798-pvrjc   1/1     Running   0          14m
userservice-7bd4fdfbb-frxm5          1/1     Running   0          14m

@bourgeoisor bourgeoisor requested review from a team and yoshi-approver as code owners April 6, 2023 22:14
@NimJay
Copy link
Collaborator

NimJay commented Apr 12, 2023

Regarding: Open question about the releasing directory

Renaming kubernetes-manifests/ to release/ makes sense to me, especially because your plan is cautious:

"this would mean keeping the kubernetes-manifests/ for a little while longer (essentially having a duplicate)"

Regarding: Open question about the releasing manifests

I think it's fine to leave as is (i.e., have duplicated database-related manifests and ConfigMaps).
The "unchanged" kubetctl apply output such as

serviceaccount/bank-of-anthos unchanged
configmap/demo-data-config unchanged

is mostly harmless.
If anything, we could revisit this as an improvement for a later release (e.g., 0.6.1).

docs/releasing/releasing.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@NimJay NimJay left a comment

Choose a reason for hiding this comment

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

Thank you again, @bourgeoisor, for the walkthrough of this pull-request yesterday.
For the open questions, I would consider opening a new GitHub issue just for tracking purposes — and work on those improvements incrementally.
I think it's important to not stuff too many changes into this pull-request.

Excellent work!
I trust your testing of the kubernetes-manifests/*.yaml. I have not tested them myself.
Approved.

@bourgeoisor bourgeoisor deleted the release/v0.6.0 branch April 24, 2023 16:07
@bourgeoisor bourgeoisor restored the release/v0.6.0 branch April 24, 2023 16:08
@NimJay NimJay mentioned this pull request Jul 18, 2023
4 tasks
@bourgeoisor bourgeoisor mentioned this pull request Nov 23, 2023
4 tasks
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.

2 participants