update for default resource tags #335

vijtrip2 · 2022-05-19T23:36:19Z

Issue #, if available: aws-controllers-k8s/community#1261

Description of changes:

Removes service.k8s.aws/managed=true and services.k8s.aws/created=%UTC_NOW% tags and adds the new services.k8s.aws/controller-version=%CONTROLLER_VERSION% tag as resource-tags input for ACK controller.
Updates ACK runtime to v0.19.0
Adds new TagConfig field inside ResourceConfig
Adds the code-generation for implementation of AWSResourceManager.EnsureTags()

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

vijtrip2 · 2022-05-19T23:38:49Z

Note to reviewer: controller-tests expected to fail due to VersionInfo refactoring from "pkg/runtime" to "pkg/types" inside ack runtime. PR -> aws-controllers-k8s/runtime#87

Locally tested that this code-generator change works as expected.

RedbackThomson · 2022-05-20T01:53:29Z

👍🏼 Will wait for the runtime PR to be merged

vijtrip2 · 2022-05-20T04:25:39Z

/hold

a-hilaly

LGTM

jaypipes

Merging this as-is. It looks like we need to update the tests to modify the go.mod files for dynamodb/ecr/s3 controllers to the being-updated-to ACK runtime version?

a-hilaly · 2022-06-08T20:51:30Z

Merging this as-is. It looks like we need to update the tests to modify the go.mod files for dynamodb/ecr/s3 controllers to the being-updated-to ACK runtime version?

@jaypipes We still don't have the part that generates EnsureTags methods for resources. This should wait for now

vijtrip2 · 2022-06-09T07:07:42Z

/unhold

vijtrip2 · 2022-06-09T07:16:34Z

Note to reviewer: DynamoDB, S3, ECR tests are expected to fail because they need an update inside their generator.yaml to ignore tagging, or provide path for the tagging field. I am planning to update the generator.yaml for these controllers when their auto generation failure issue gets created by ack-bot.

Failures in this PR are because the default top level Tags field does not exist inside Backup(Dynamo), Bucket(S3) and PullThroughCache(ECR) resources.

pkg/generate/code/initialize_field.go

pkg/model/crd.go

pkg/testdata/models/apis/s3/0000-00-00/generator-with-tags.yaml

templates/pkg/resource/manager.go.tpl

jaypipes

great start, @vijtrip2! few notes inline but overall looking good.

pkg/generate/code/initialize_field.go

templates/pkg/resource/manager.go.tpl

templates/pkg/resource/tags.go.tpl

RedbackThomson

Just had 2 more nits. But I think the functionality and logic is all solid.

pkg/generate/code/initialize_field.go

templates/pkg/resource/tags.go.tpl

ack-bot · 2022-06-09T22:48:49Z

@vijtrip2: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
s3-olm-test	`4900b99`	link	`/test s3-olm-test`
dynamodb-controller-test	`4900b99`	link	`/test dynamodb-controller-test`
s3-controller-test	`4900b99`	link	`/test s3-controller-test`
ecr-controller-test	`4900b99`	link	`/test ecr-controller-test`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

jaypipes

Rock on brother Vijay :) Really smooth work. ++

jaypipes · 2022-06-10T00:46:50Z

pkg/generate/code/initialize_field.go

+			} else {
+				elemAccessPrefix = fmt.Sprintf("%s%s", elemAccessPrefix,
+					r.Config().PrefixConfig.StatusField)
+			}


jaypipes · 2022-06-10T00:58:57Z

/lgtm

ack-bot · 2022-06-10T00:59:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, jaypipes, RedbackThomson, vijtrip2

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [A-Hilaly,RedbackThomson,jaypipes,vijtrip2]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

updated the default resource tags

7032235

vijtrip2 requested review from jaypipes and RedbackThomson May 19, 2022 23:36

ack-bot requested review from a-hilaly and brycahta May 19, 2022 23:36

ack-bot added the approved label May 19, 2022

ack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 20, 2022

Upgrade to runtime v0.19.0

88aa3e1

a-hilaly approved these changes Jun 8, 2022

View reviewed changes

jaypipes approved these changes Jun 8, 2022

View reviewed changes

jaypipes added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 8, 2022

vijtrip2 added 6 commits June 8, 2022 23:58

update ACK runtime to v0.19.0

9371653

update ResourceConfig to include TagsConfig

4f2d4f0

add tagging methods inside model.CRD

8e01e12

add new generator method to initialize nested structs

0f0f9ff

update templates to ensure ack tags

9c2e73d

document the hooks for ensuring tags

21df70e

ack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 9, 2022

vijtrip2 requested review from jaypipes and a-hilaly June 9, 2022 07:16

brycahta reviewed Jun 9, 2022

View reviewed changes

pkg/generate/code/initialize_field.go Show resolved Hide resolved

pkg/model/crd.go Outdated Show resolved Hide resolved

pkg/model/crd.go Outdated Show resolved Hide resolved

refactor model.GetIgnoreTagging -> config.TagsAreIgnored(resName)

3ac2ae1

vijtrip2 requested a review from brycahta June 9, 2022 18:20

RedbackThomson suggested changes Jun 9, 2022

View reviewed changes

pkg/model/crd.go Outdated Show resolved Hide resolved

pkg/testdata/models/apis/s3/0000-00-00/generator-with-tags.yaml Outdated Show resolved Hide resolved

templates/pkg/resource/manager.go.tpl Show resolved Hide resolved

jaypipes requested changes Jun 9, 2022

View reviewed changes

vijtrip2 added 2 commits June 9, 2022 13:41

refactor crd.Tagging methods into pkg/model/tagging.go

623f7e9

determing Spec/Status prefix from inspecting the field

cc38201

vijtrip2 requested review from jaypipes and RedbackThomson June 9, 2022 21:46

RedbackThomson approved these changes Jun 9, 2022

View reviewed changes

pkg/generate/code/initialize_field.go Show resolved Hide resolved

templates/pkg/resource/tags.go.tpl Show resolved Hide resolved

remove trailing newline from tags.go.tpl

4900b99

jaypipes approved these changes Jun 10, 2022

View reviewed changes

ack-bot assigned jaypipes Jun 10, 2022

ack-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2022

ack-bot merged commit 160b839 into aws-controllers-k8s:main Jun 10, 2022

jaypipes mentioned this pull request Jun 27, 2022

Consistent Tags resource handling aws-controllers-k8s/community#63

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update for default resource tags #335

update for default resource tags #335

vijtrip2 commented May 19, 2022 •

edited

Loading

vijtrip2 commented May 19, 2022 •

edited

Loading

RedbackThomson commented May 20, 2022

vijtrip2 commented May 20, 2022

a-hilaly left a comment

jaypipes left a comment

a-hilaly commented Jun 8, 2022

vijtrip2 commented Jun 9, 2022

vijtrip2 commented Jun 9, 2022

jaypipes left a comment

RedbackThomson left a comment

ack-bot commented Jun 9, 2022

jaypipes left a comment

jaypipes Jun 10, 2022

jaypipes commented Jun 10, 2022

ack-bot commented Jun 10, 2022

update for default resource tags #335

update for default resource tags #335

Conversation

vijtrip2 commented May 19, 2022 • edited Loading

vijtrip2 commented May 19, 2022 • edited Loading

RedbackThomson commented May 20, 2022

vijtrip2 commented May 20, 2022

a-hilaly left a comment

Choose a reason for hiding this comment

jaypipes left a comment

Choose a reason for hiding this comment

a-hilaly commented Jun 8, 2022

vijtrip2 commented Jun 9, 2022

vijtrip2 commented Jun 9, 2022

jaypipes left a comment

Choose a reason for hiding this comment

RedbackThomson left a comment

Choose a reason for hiding this comment

ack-bot commented Jun 9, 2022

jaypipes left a comment

Choose a reason for hiding this comment

jaypipes Jun 10, 2022

Choose a reason for hiding this comment

jaypipes commented Jun 10, 2022

ack-bot commented Jun 10, 2022

vijtrip2 commented May 19, 2022 •

edited

Loading

vijtrip2 commented May 19, 2022 •

edited

Loading