CFE-684: Add user defined tags to the created gcp resource #54

bharath-b-rh · 2023-06-27T07:52:04Z

PR has the changes for enhancement proposed to support GCP tags in OCP, which requires machine-api-provider-gcp to add gcp userTags available in the status sub resource of infrastructure CR, to all the gcp resources created by the operator.

bharath-b-rh · 2023-06-27T07:52:17Z

/hold openshift/enhancements#1217

openshift-ci · 2023-06-27T07:52:40Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci-robot · 2023-08-09T10:43:15Z

@bharath-b-rh: This pull request references CFE-684 which is a valid jira issue.

In response to this:

PR has the changes for enhancement proposed to support GCP tags in OCP, which requires machine-api-provider-gcp to add gcp userTags available in the status sub resource of infrastructure CR, to all the gcp resources created by the operator.

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.

openshift-bot · 2023-11-09T09:01:03Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

bharath-b-rh · 2023-11-27T15:01:00Z

/hold openshift/api#1506

bharath-b-rh · 2023-11-27T15:02:11Z

/remove-label lifecycle/stale

openshift-ci · 2023-11-27T15:03:06Z

@bharath-b-rh: The label(s) /remove-label lifecycle/stale cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, downstream-change-needed, rebase/manual, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, jira/valid-bug, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/remove-label lifecycle/stale

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.

bharath-b-rh · 2023-11-27T15:03:21Z

/remove-lifecycle stale

bharath-b-rh · 2023-11-27T15:11:45Z

/cc @JoelSpeed

bharath-b-rh · 2024-01-11T08:41:58Z

/retest

theobarberbany

Looks good after a first pass! I have a few questions / nits :)

cmd/manager/main.go

pkg/cloud/gcp/actuators/machine/machine_scope.go

pkg/cloud/gcp/actuators/machine/reconciler_test.go

pkg/cloud/gcp/actuators/machine/reconciler.go

pkg/cloud/gcp/actuators/util/gcp_tags_labels_test.go

pkg/cloud/gcp/actuators/util/gcp_tags_labels.go

bharath-b-rh · 2024-01-30T07:14:06Z

/remove-hold

pkg/cloud/gcp/actuators/machine/machine_scope.go

pkg/cloud/gcp/actuators/util/gcp_tags_labels.go

pkg/cloud/gcp/actuators/machine/reconciler.go

pkg/cloud/gcp/actuators/services/tags/tagservice.go

pkg/cloud/gcp/actuators/services/tags/tagservice_mock.go

pkg/cloud/gcp/actuators/util/gcp_tags_labels.go

pkg/cloud/gcp/actuators/machine/reconciler.go

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

openshift-ci · 2024-02-16T06:49:40Z

@bharath-b-rh: all tests passed!

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.

theobarberbany · 2024-03-14T14:02:34Z

/lgtm

My only nit would be potentially generating some of the test cases (e.g a for loop rather than tens of tags being defined as literals)

Nice job!

elmiko

this mostly makes sense to me, just had a question about the unit tests

elmiko · 2024-03-14T13:55:31Z

pkg/cloud/gcp/actuators/machine/machine_scope_test.go

@@ -215,6 +217,8 @@ func TestNewMachineScope(t *testing.T) {
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			gs := NewWithT(t)
+			tc.params.tagsClientBuilder = tagservice.NewMockTagServiceBuilder
+			tc.params.featureGates = featuregates.NewFeatureGate(nil, []configv1.FeatureGateName{configv1.FeatureGateGCPLabelsTags})


i would have expected us to add new test cases to exercise the tab builder service when the feature gate is on or off (to make sure the negative assertion works), is this already captured in the tests?

i'm curious why we are always setting the feature gate, eg shouldn't we have a test for creation with the feature gate enabled and one with it disabled?

Yeah, both scenarios are covered but not in same modules. Test cases added in actuator and machine_scope modules have the featuregate disabled and cases in reconciler have it enabled. And test cases added for gcp_tags modules is for enabled scenario.

elmiko

/approve

openshift-ci · 2024-03-28T20:24:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elmiko

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~~ [elmiko]

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

openshift-bot · 2024-03-29T00:15:45Z

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-machine-api-provider-gcp-container-v4.16.0-202403281943.p0.gfc80687.assembly.stream.el9 for distgit ose-machine-api-provider-gcp.
All builds following this will include this PR.

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 27, 2023

bharath-b-rh force-pushed the cfe-684 branch from 6f394f3 to 1768927 Compare July 3, 2023 09:27

bharath-b-rh force-pushed the cfe-684 branch 2 times, most recently from 082acf6 to fd54b9b Compare July 25, 2023 08:03

bharath-b-rh changed the title ~~CFE-684 : Add user defined tags to the created gcp resource~~ CFE-684: Add user defined tags to the created gcp resource Aug 9, 2023

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 9, 2023

openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 9, 2023

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 9, 2023

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2023

bharath-b-rh force-pushed the cfe-684 branch from 470f119 to bd17b92 Compare November 24, 2023 09:23

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 24, 2023

bharath-b-rh force-pushed the cfe-684 branch from bd17b92 to 9f7fd2a Compare November 27, 2023 14:59

bharath-b-rh marked this pull request as ready for review November 27, 2023 15:01

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 27, 2023

openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 27, 2023

openshift-ci bot requested review from elmiko and nrb November 27, 2023 15:11

openshift-ci bot requested a review from JoelSpeed November 27, 2023 15:12

bharath-b-rh force-pushed the cfe-684 branch from 9f7fd2a to abcee33 Compare December 16, 2023 11:02

theobarberbany reviewed Jan 12, 2024

View reviewed changes

bharath-b-rh force-pushed the cfe-684 branch 2 times, most recently from 6ce7fe4 to e36eb83 Compare January 23, 2024 06:19

JoelSpeed reviewed Jan 23, 2024

View reviewed changes

bharath-b-rh force-pushed the cfe-684 branch 2 times, most recently from df8eab3 to 18602b1 Compare January 30, 2024 07:07

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 30, 2024

JoelSpeed reviewed Jan 31, 2024

View reviewed changes

pkg/cloud/gcp/actuators/machine/machine_scope.go Outdated Show resolved Hide resolved

pkg/cloud/gcp/actuators/util/gcp_tags_labels.go Outdated Show resolved Hide resolved

pkg/cloud/gcp/actuators/machine/reconciler.go Outdated Show resolved Hide resolved

bharath-b-rh force-pushed the cfe-684 branch from 18602b1 to 002bb45 Compare February 7, 2024 15:21

JoelSpeed reviewed Feb 13, 2024

View reviewed changes

bharath-b-rh force-pushed the cfe-684 branch 2 times, most recently from d7f6515 to 34c53ff Compare February 14, 2024 05:13

JoelSpeed reviewed Feb 14, 2024

View reviewed changes

pkg/cloud/gcp/actuators/util/gcp_tags_labels.go Show resolved Hide resolved

CFE-684: Add user defined tags to the created gcp resources

46a0e1a

bharath-b-rh force-pushed the cfe-684 branch from 34c53ff to 46a0e1a Compare February 14, 2024 10:49

JoelSpeed reviewed Feb 14, 2024

View reviewed changes

pkg/cloud/gcp/actuators/machine/reconciler.go Outdated Show resolved Hide resolved

Apply suggestions from code review

a452397

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

openshift-ci bot assigned theobarberbany Mar 14, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 14, 2024

elmiko reviewed Mar 14, 2024

View reviewed changes

elmiko reviewed Mar 28, 2024

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 28, 2024

openshift-merge-bot bot merged commit fc80687 into openshift:main Mar 28, 2024
9 checks passed

bharath-b-rh deleted the cfe-684 branch April 1, 2024 05:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CFE-684: Add user defined tags to the created gcp resource #54

CFE-684: Add user defined tags to the created gcp resource #54

bharath-b-rh commented Jun 27, 2023

bharath-b-rh commented Jun 27, 2023

openshift-ci bot commented Jun 27, 2023

openshift-ci-robot commented Aug 9, 2023 •

edited by openshift-ci bot

openshift-bot commented Nov 9, 2023

bharath-b-rh commented Nov 27, 2023

bharath-b-rh commented Nov 27, 2023

openshift-ci bot commented Nov 27, 2023

bharath-b-rh commented Nov 27, 2023

bharath-b-rh commented Nov 27, 2023

bharath-b-rh commented Jan 11, 2024

theobarberbany left a comment

bharath-b-rh commented Jan 30, 2024

openshift-ci bot commented Feb 16, 2024

theobarberbany commented Mar 14, 2024 •

edited

elmiko left a comment

elmiko Mar 14, 2024

bharath-b-rh Mar 15, 2024

elmiko left a comment

openshift-ci bot commented Mar 28, 2024

openshift-bot commented Mar 29, 2024

CFE-684: Add user defined tags to the created gcp resource #54

CFE-684: Add user defined tags to the created gcp resource #54

Conversation

bharath-b-rh commented Jun 27, 2023

bharath-b-rh commented Jun 27, 2023

openshift-ci bot commented Jun 27, 2023

openshift-ci-robot commented Aug 9, 2023 • edited by openshift-ci bot

openshift-bot commented Nov 9, 2023

bharath-b-rh commented Nov 27, 2023

bharath-b-rh commented Nov 27, 2023

openshift-ci bot commented Nov 27, 2023

bharath-b-rh commented Nov 27, 2023

bharath-b-rh commented Nov 27, 2023

bharath-b-rh commented Jan 11, 2024

theobarberbany left a comment

Choose a reason for hiding this comment

bharath-b-rh commented Jan 30, 2024

openshift-ci bot commented Feb 16, 2024

theobarberbany commented Mar 14, 2024 • edited

elmiko left a comment

Choose a reason for hiding this comment

elmiko Mar 14, 2024

Choose a reason for hiding this comment

bharath-b-rh Mar 15, 2024

Choose a reason for hiding this comment

elmiko left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Mar 28, 2024

openshift-bot commented Mar 29, 2024

openshift-ci-robot commented Aug 9, 2023 •

edited by openshift-ci bot

theobarberbany commented Mar 14, 2024 •

edited