Skip to content

Commit

Permalink
Merge branch 'GoogleCloudPlatform:main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
etruong42 authored May 15, 2024
2 parents 2b3575d + f5fb3aa commit e91601c
Show file tree
Hide file tree
Showing 7,259 changed files with 504,944 additions and 214,975 deletions.
The diff you're trying to view is too large. We only load the first 3000 changed files.
22 changes: 15 additions & 7 deletions .ci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,17 @@ Don't panic - this is all quite safe and we have fixed it before. We store the

It's possible for a job to be cancelled or fail in the middle of pushing downstreams in a transient way. The sorts of failures that happen at scale - lightning strikes a datacenter (ours or GitHub's!) or some other unlikely misfortune happens. This has a chance to cause a hiccup in the downstream history, but isn't dangerous. If that happens, the sync tags may need to be manually updated to sit at the same commit, just before the commit which needs to be generated, or some failed tasks might need to be run by hand.

Updating the sync tags is done like this:
First, check their state: `git fetch origin && git rev-parse origin/tpg-sync origin/tpgb-sync origin/tf-oics-sync origin/tf-validator-sync` will list the commits for each of the sync tags.
(If you have changed the name of the `googlecloudplatform/magic-modules` remote from `origin`, substitute that name instead)
First, check their state: `git fetch origin && git rev-parse origin/tpg-sync origin/tpgb-sync origin/tf-oics-sync origin/tgc-sync` will list the commits for each of the sync tags. (If you have changed the name of the `GoogleCloudPlatform/magic-modules` remote from `origin`, substitute that name instead, such as `git fetch upstream && git rev-parse upstream/tpg-sync upstream/tpgb-sync upstream/tf-oics-sync upstream/tgc-sync`)

In normal, steady-state operation, these tags will all be identical. When a failure occurs, some of them may be one commit ahead of the others. It is rare for any of them to be 2 or more commits ahead of any other. If some of them are one commit ahead of the others, and there is no pusher task currently running, this means you need to reset them by hand and rerun the failed jobs. If they diverge by more than one commit, or a pusher task is currently running, you will need to manually run missing tasks.

### Divergence by zero commits

Just click retry on the failed job in Cloud Build. Yay!
Just click retry on the failed job in Cloud Build. This is fairly rare, as most failures involve a step failing after another has already succeeded.

### Divergence by exactly one commit.

Find which commit caused the error. This will usually be easy - cloud build lists the commit which triggered a build, so you can probably just use that one. You need to set all the sync tags to the parent of that commit. Say the commit which caused the error is `12345abc`. You can find the parent of that commit with `git rev-parse 12345abc~` (note the `~` suffix). Some of the sync tags are likely set to this value already. For the remainder, simply perform a git push. Assuming that the parent commit is `98765fed`, that would be, e.g. `git push origin 98765fed:tf-validator-sync`.
Find which commit caused the error. This will usually be easy - cloud build lists the commit which triggered a build, so you can probably just use that one. You need to set all the sync tags to the parent of that commit. Say the commit which caused the error is `12345abc`. You can find the parent of that commit with `git rev-parse 12345abc~` (note the `~` suffix). Some of the sync tags are likely set to this value already. For the remainder, simply perform a git push. Assuming that the parent commit is `98765fed`, that would be, e.g. `git push -f origin 98765fed:tf-validator-sync`.

If you are unlucky, there may be open PRs - this only happens if the failure occurred during the ~5 second period surrounding the merging of one of the downstreams. Close those PRs before proceeding to the final step.

Expand All @@ -73,7 +72,7 @@ VERSION=beta
git clone https://github.com/GoogleCloudPlatform/magic-modules fix-gcb-run
pushd fix-gcb-run
docker pull gcr.io/graphite-docker-images/downstream-builder;
for commit in $(git log $SYNC_TAG..main --pretty=%H | tac); do
for commit in $(git log $SYNC_TAG..main --pretty=%H | tac); do
git checkout $commit && \
docker run -v `pwd`:/workspace -w /workspace -e GITHUB_TOKEN=$MAGICIAN_GITHUB_TOKEN -it gcr.io/graphite-docker-images/downstream-builder downstream $REPO $VERSION $commit || \
break;
Expand All @@ -82,21 +81,30 @@ done

In the event of a failure, this will stop running. If it succeeds, update the sync tag with `git push origin HEAD:tpg-sync`.

### Downstream build job is not triggered by commits.
This is rare but we've seen this happened before. In this case, we need to manually trigger a Cloud Build job by running
```
gcloud builds triggers run build-downstreams --project=graphite-docker-images --substitutions=BRANCH_NAME=<BASE_BRANCH_NAME> --sha=<COMMIT_SHA>
```
You'll need to substitute `<COMMIT_SHA>` with the commit sha that you'd like to trigger the build against and `<BASE_BRANCH_NAME>` with base branch that this commit is pushed into, likely `main` but could be feature branches in some cases.

## Deploying the pipeline
The code on the PR's branch is used to plan actions - no merge is performed.
If you are making changes to the workflows, your changes will not trigger a workflow run, because of the risk of an untrusted contributor introducing malicious code in this way. You will need to test locally by using the [cloud build local builder](https://cloud.google.com/cloud-build/docs/build-debug-locally).
If you are making changes to the containers, your changes will not apply until they are merged in and built - this can take up to 15 minutes. If you need to make a breaking change, you will need to pause the pipeline while the build happens. If you are making changes to both the containers and the workflows and those changes need to be coordinated, you will need to pause the build while the containers build and enforce every open PR be rebased on top of our PR. It is probably better to build in backwards-compatibility into your containers. We recommend a 14 day window - 14 days after your change goes in, you can remove the backwards-compatibility.

Pausing the pipeline is done in the cloud console, by setting the downstream-builder trigger to disabled. You can find that trigger [here](https://console.cloud.google.com/cloud-build/triggers/edit/f80a7496-b2f4-4980-a706-c5425a52045b?project=graphite-docker-images)


## Dependency change handbook:
If someone (often a bot) creates a PR which updates Gemfile or Gemfile.lock, they will not be able to generate diffs. This is because bundler doesn't allow you to run a binary unless your installed gems exactly match the Gemfile.lock, and since we have to run generation before and after the change, there is no possible container that will satisfy all requirements.

The best approach is
* Build the `downstream-generator` container locally, with the new Gemfile and Gemfile.lock. This will involve hand-modifying the Dockerfile to use the local Gemfile/Gemfile.lock instead of wget from this repo's `main` branch. You don't need to check in those changes.
* When that container is built, and while nothing else is running in GCB (wait, if you need to), push the container to GCR, and as soon as possible afterwards, merge the dependency-changing PR.

## Changes to cloud build yaml:
If changes are made to `gcb-contributor-membership-checker.yml` or `gcb-community-checker.yml` they will not be reflected in presubmit runs for existing PRs without a rebase. This is because these build triggers are linked to pull request creation and not pushes to the PR branch. If changes are needed to these build files they will need to be made in a backwards-compatible manner. Note that changes to other files used by these triggers will be immediately reflected in all PRs, leading to a possible disconnect between the yaml files and the rest of the CI code.

## Historical Note: Design choices & tradeoffs
* The downstream push doesn't wait for checks on its PRs against downstreams. This may inconvenience some existing workflows which rely on the downstream PR checks. This ensures that merge conflicts never come into play, since the downstreams never have dangling PRs, but it requires some up-front work to get those checks into the differ. If a new check is introduced into the downstream Travis, we will need to introduce it into the terraform-tester container.
* The downstream push is disconnected from the output of the differ (but runs the same code). This means that the diff which is approved isn't guaranteed to be applied *exactly*, if for instance magic modules' behavior changes on main between diff generation and downstream push. This is also intended to avoid merge conflicts by, effectively, rebasing each commit on top of main before final generation is done.
Expand Down
281 changes: 2 additions & 279 deletions .ci/RELEASE_NOTES_GUIDE.md
Original file line number Diff line number Diff line change
@@ -1,281 +1,4 @@
# Release Notes for Terraform/Magic Modules Auto-CHANGELOG
# Terraform Release Notes Guide

## Background
This guide has been moved to https://googlecloudplatform.github.io/magic-modules/contribute/release-notes/. This file will be deleted in the future.

The Magician bot has the ability to copy specifically formatted release notes
from upstream Magic-Modules PRs to downstream PRs that have CHANGELOGS, namely
PRs generated in the Terraform providers (GA, beta).

Code lives in magic-modules/downstream_changelog_metadata.py

This guide discusses the style and format of release notes to add
in PR descriptions so they will be copied downstream and used
in CHANGELOGs.

## Expected Format

The description should have Markdown-formatted code blocks with language
headings (i.e. text right after the three ticks) like this:

~~~
PR Description
...
```release-note:enhancement
compute: Fixed permadiff for `bar` in `google_compute_foo`
```
~~~

You can have multiple code blocks to have multiple release notes per PR, i.e.

~~~
PR Description
...
```release-note:deprecation
container: Deprecated `region` for `google_container_cluster` - use location instead.
```
```release-note:enhancement
container: Added general field `location` to `google_container_cluster`
```
~~~


Do not indent the block and make sure to leave newlines, so you don't confuse
the Markdown parser.

To qualify that a change is specific to the beta provider add `(beta)`
at the end of the release note.\
This will omit the note from changelogs for the ga release.

~~~
```release-note:enhancement
compute: added field `multiwriter` to resource `disk` (beta)
```
~~~


## Headings

Release notes should be formatted with one of the following headings:
- `release-note:enhancement`
- `release-note:bug`
- `release-note:note`
- `release-note:new-resource`
- `release-note:new-datasource`
- `release-note:deprecation`
- `release-note:breaking-change`
- `release-note:none`

However, any note with a language heading starting with ```release-note:... will get copied.

## Non-User-Facing PRs

Any PR that should not have any impact on users (test fixes, code generation, website updates,
CI changes, etc.) should use a `release-note:none` block. It can be left empty, or can be
optionally filled with an explanation of why the PR is not user-facing.

By including this block explicitly, it lets whoever is generating the changelog know that
a release note was explicitly omitted, not just forgotten. It'll also let your PR pass any
future automation around release note correctness checking.

## Release Note Style Guide (Terraform-specific)

Notes SHOULD:
- Start with a verb
- Use past tense (added/fixed/resolved) as much as possible
- Only use present tense to suggest future behavior, i.e. for breaking
changes, deprecations, or new behavior.
- Impersonal third person (no “I”, “you”, etc.)
- Start with {{service}} if changing an existing resource (see below)

Notes, breaking changes, and features are exceptions. These are more free-form and left to
the discretion of the PR author and reviewer. The overarching goal should be a good user
experience when reading the changelog.

See examples below for good release notes.

### Examples:

**Enhancements:** adding fields or new features to existing resources

~~~
```release-note:enhancement
compute: added `foo_bar` field to `google_compute_foo` resource
```
~~~
**Bugs:** fixing existing resources

~~~
```release-note:bug
container: fixed perma-diff in `google_container_cluster`
```
~~~

**Breaking Changes:** changes to existing resources that may require users to change their config

~~~
```release-note:breaking-change
project: made `iam_policy` authoritative
```
~~~

**Deprecations:** Announce deprecations when fields/resources are marked as deprecated, not removed

~~~
``` release-note:deprecation
container: deprecated `region` and `zone` on `google_container_unicorn`. Use `location` instead.
```
~~~

**New Resources And Datasources:**
(note no service name or *New Resource* tag)

~~~
```release-note:new-resource
`google_compute_new_resource`
```
~~~

~~~
```release-note:new-datasource
`google_compute_new_datasource`
```
~~~

**Notes:** General tag for things that don’t have changes in provider but may be important to users. Syntax is slightly more flexible here.

```release-note:note
Starting on Nov 1, 2019, Cloud Functions API will be private by default. Add appropriate bindings through `google_cloud_function_iam_*` resources to manage privileges for `google_cloud_function` resources created by Terraform.
```
### Counter-examples:

The following changelog entries are not ideal.

#### No Type

~~~
```release-note:REPLACEME
compute: fixed permadiff on description for `google_compute_instance`
```
~~~

This doesn't update the type of release note, which means it will need to be corrected at generation time.

Better:

~~~
```release-note:bug
compute: fixed permadiff on description for `google_compute_instance`
```
~~~

### Not Past Tense

~~~
```release-note:bug
compute: Fix permadiff on description for `google_compute_instance`
```
~~~

This doesn't use the past tense. Readers of the changelog will be reading what _happened_ in a release,
so the language should be that of describing what happened. Imagine you're answering the question
"what changed since the last version?"

Better:

~~~
```release-note:bug
compute: Fixed permadiff for `google_compute_instance`
```
~~~

### No Service

~~~
```release-note:bug
Fixed permadiff on description for `google_compute_instance`
```
~~~

This doesn't start with a service name. By convention, we prefix all our bug and enhancement changelog
entries with service names, and the other entries when it makes sense and seems beneficial. This helps
sort the changelog and group related changes together, and helps users scan for the services they use.

Better:

~~~
```release-note:bug
compute: Fixed permadiff on description for `google_compute_instance`
```
~~~

### Not User-Centric

~~~
```release-note:bug
compute: made description Computed for `google_compute_instance`
```
~~~

This isn't written for the right audience; our users don't all, or even mostly, know what Computed
means, and shouldn't have to. Instead, describe the impact that this will have on them.

Better:

~~~
```release-note:bug
compute: fixed permadiff on description for `google_compute_instance`
```
~~~

### Resource Instead of Service

~~~
```release-note:bug
compute_instance: Fixed permadiff on description for `google_compute_instance`
```
~~~

This uses the resource instead of the service as a prefix.

Better:

~~~
```release-note:bug
compute: Fixed permadiff on description for `google_compute_instance`
```
~~~

Choosing the right service name is a bit of an art. A good rule of thumb is if there's something
besides the resource name after `google_`, use that. For example, `compute` is a good choice from
`google_compute_instance`. Not every resource has that, however; for `google_project`, the service
is not part of the resource address. In these cases, falling back on the name of the package the
resource's APIs is implemented in (`resourcemanager`, for `google_project`) is a good call.

Not every change applies only to one resource. Judgment is best here. When in doubt, `provider` is
a good way to indicate sweeping changes that are likely to impact most users.

### Unticked Resource Names

~~~
```release-note:bug
compute: Fixed permadiff on description for google_compute_instance
```
~~~

This doesn't have \`\` marks around the resource name, which by convention we do. This sets the resource
name apart, making it easer to notice.

Better:

~~~
```release-note:bug
compute: Fixed permadiff on description for `google_compute_instance`
```
~~~
28 changes: 19 additions & 9 deletions .ci/containers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,27 @@ The docker images located in this folder are used by multiple builds for magic m

## Naming Convention

The images are named with the languages they contain and the images are versioned with tags that indicate the version of each language contained. eg: the image `go-ruby-python` with a tag of `1.11.5-2.6.0-2.7` indicates that the image has `go 1.11.5`, `ruby 2.6.0` and `python 2.7`.
The images are named according to their use. We have a small number of images that get reused in multiple places, based around sets of requirements shared by different parts of the build pipeline. The images are:

If there are multiple images with the same language version but different libraries (gems), a `v#` is appended to differentiate. eg: `1.11.5-2.6.0-2.7-v6`
- `gcr.io/graphite-docker-images/bash-plus`
- `gcr.io/graphite-docker-images/build-environment`
- `gcr.io/graphite-docker-images/go-plus`

## Updating a docker image
The Dockerfile should be updated, then the image rebuilt and pushed to the container registry stored at the `graphite-docker-images` GCP project. To update any of the images:

Before you begin, set up Docker (including configuring it to [authenticate with gcloud](https://cloud.google.com/container-registry/docs/advanced-authentication#gcloud-helper)).

1. Make changes to the Dockerfile
2. Configure docker to use gcloud auth: `gcloud auth configure-docker`
3. Build the image: `docker build . --tag gcr.io/graphite-docker-images/go-ruby-python`
4. Find the new image's id: `docker images`
5. Add the appropriate tag `docker tag ac37c0af8ce7 gcr.io/graphite-docker-images/go-ruby-python:1.11.5-2.6.0-2.7-v6`
6. Push the image: `docker push gcr.io/graphite-docker-images/go-ruby-python:1.11.5-2.6.0-2.7-v6`
7. Check the UI and ensure the new version is available and tagged at `latest`. It must be tagged `latest` for the Kokoro builds to get the correct version.
2. Build the image with the `testing` tag:
```bash
sudo docker build . --tag gcr.io/graphite-docker-images/bash-plus:testing
```
3. Push the image:
```bash
sudo docker push gcr.io/graphite-docker-images/bash-plus:testing
```
4. Update cloudbuild yaml files to reference the image you just pushed by adding the `:testing` suffix
5. Update files that will cause the cloudbuild yaml changes (and therefore your changes) to be exercised
- Tip: Modifying `mmv1/third_party/terraform/services/compute/metadata.go.erb` will trigger builds for TPG, TPGB, and TGC.
6. Create a PR with these changes.
7. Verify that the cloudbuild steps that should use your testing image _are_ using your testing image (in the Execution Details tab for the step.)
5 changes: 5 additions & 0 deletions .ci/containers/bash-plus/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
FROM alpine/git

RUN apk update && apk upgrade && \
apk add --no-cache bash jq curl && \
rm -rf /var/cache/apk/*
Loading

0 comments on commit e91601c

Please sign in to comment.