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

Cleanup terraformer usage #208

Merged
merged 3 commits into from
Nov 27, 2020
Merged

Conversation

timebertt
Copy link
Member

@timebertt timebertt commented Nov 25, 2020

How to categorize this PR?

/area logging quality
/kind cleanup
/priority normal
/platform gcp

What this PR does / why we need it:

This PR adapts the infrastructure actuator to gardener/gardener#3223 by

  • correctly propagating contexts in terraformer calls
  • switching to logr for terraformer usage (instead of logrus) which makes it consistent with the other controller logs (which already use logr). Now all the logging in the infrastructure controller/actuator is done via logr instead of a mix between logr and logrus.

Also, it improves logging in the infrastructure actuator and test by making extensive use of the structured logging mechanisms (setting additional variables at the top level functions, which will be propagated to all logs - even to the terraformer logs), so they will be more helpful for debugging future issues.

For example:

Before:

2020-11-25T13:42:35.227+0100	INFO	infrastructure_controller	Deleting the infrastructure	{"infrastructure": "infrastructure"}
2020-11-25T13:42:35.227+0100	DEBUG	controller-runtime.manager.events	Normal	{"object": {"kind":"Infrastructure","namespace":"gcp-infrastructure-it--n93ux","name":"infrastructure","uid":"e6dd22b6-d36c-490d-8d62-a5f37f7ecd4f","apiVersion":"extensions.gardener.cloud/v1alpha1","resourceVersion":"24386"}, "reason": "InfrastructureDeletion", "message": "Deleting the infrastructure"}
time="2020-11-25T13:42:39+01:00" level=info msg="Deploying Terraformer Pod with .meta.generateName 'infrastructure.infra.tf-destroy-'."
time="2020-11-25T13:42:39+01:00" level=info msg="Successfully created Terraformer Pod 'infrastructure.infra.tf-destroy-dkqrc'."
time="2020-11-25T13:42:39+01:00" level=info msg="Waiting for Terraform Pod 'infrastructure.infra.tf-destroy-dkqrc' to be completed..."

Now:

2020-11-25T11:48:18.075+0100	INFO	infrastructure_controller	Deleting the infrastructure	{"infrastructure": "gcp-infrastructure-it--9xl55/infrastructure", "operation": "delete"}
2020-11-25T11:48:18.075+0100	DEBUG	controller-runtime.manager.events	Normal	{"object": {"kind":"Infrastructure","namespace":"gcp-infrastructure-it--9xl55","name":"infrastructure","uid":"8226ee87-dd61-4009-a27b-83bcd61d20a3","apiVersion":"extensions.gardener.cloud/v1alpha1","resourceVersion":"10943"}, "reason": "InfrastructureDeletion", "message": "Deleting the infrastructure"}
2020-11-25T11:48:18.394+0100	INFO	infrastructure-actuator.terraformer	Ensuring all terraformer Pods have been deleted	{"infrastructure": "gcp-infrastructure-it--9xl55/infrastructure", "operation": "delete"}
2020-11-25T11:48:18.399+0100	INFO	infrastructure-actuator.terraformer	Waiting for clean environment	{"infrastructure": "gcp-infrastructure-it--9xl55/infrastructure", "operation": "delete"}

Special notes for your reviewer:
✅ depends on gardener/gardener#3223

Release note:

Logging in the infrastructure actuator has been improved to make it consistent in the logging format and more readable/helpful.

@gardener-robot gardener-robot added area/logging Logging related area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/cleanup Something that is not needed anymore and can be cleaned up platform/gcp Google cloud platform/infrastructure priority/normal needs/review Needs review size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Nov 25, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 25, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 25, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 25, 2020
@rfranzke
Copy link
Member

/test-single

@testmachinery
Copy link

testmachinery bot commented Nov 25, 2020

🔥 Oops, something went wrong @rfranzke
unable to create Testrun

Command test-single
Runs a single testrun that is either specified by command line flag or in the default values
The specified path is rendered as testrun and the current repository is injected as a default location.


Example: /test-single [path to the testrun]

Usage:
      --dry-run   Print the rendered testrun

Instructions for interacting with me using PR comments are available here

@rfranzke
Copy link
Member

/assign @schrodit
👀

@schrodit
Copy link

🔥 Oops, something went wrong @rfranzke
unable to create Testrun

Instructions for interacting with me using PR comments are available here

we are running again into rate limits: 403 API rate limit exceeded for user ID 499 85316

fyi @dguendisch

@gardener-robot gardener-robot added size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) and removed size/l Size of pull request is large (see gardener-robot robot/bots/size.py) labels Nov 26, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 26, 2020
@timebertt
Copy link
Member Author

Vendored the current g/g master, so as such this PR is
/ready
But we can also choose not to vendor a master and update the PR once g/g is released. Opinions?

/test-single

@testmachinery
Copy link

testmachinery bot commented Nov 26, 2020

Testrun: e2e-lkr6r
Workflow: e2e-lkr6r-wf
Phase: Succeeded

+---------------------+---------------------+-----------+----------+
|        NAME         |        STEP         |   PHASE   | DURATION |
+---------------------+---------------------+-----------+----------+
| infrastructure-test | infrastructure-test | Succeeded | 7m55s    |
+---------------------+---------------------+-----------+----------+

@gardener-robot gardener-robot marked this pull request as ready for review November 26, 2020 06:48
@gardener-robot gardener-robot requested review from a team as code owners November 26, 2020 06:48
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 26, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 26, 2020
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review needs/second-opinion Needs second review by someone else labels Nov 27, 2020
Copy link

@prashanth26 prashanth26 left a comment

Choose a reason for hiding this comment

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

/lgtm

@timebertt timebertt merged commit 89f8007 into gardener:master Nov 27, 2020
@timebertt timebertt deleted the cleanup/terraformer branch November 27, 2020 08:00
@gardener-robot gardener-robot added priority/3 Priority (lower number equals higher priority) and removed priority/3 Priority (lower number equals higher priority) labels Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/logging Logging related area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/cleanup Something that is not needed anymore and can be cleaned up needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) platform/gcp Google cloud platform/infrastructure reviewed/lgtm Has approval for merging size/m Size of pull request is medium (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants