CFE-1130: AWS DAY2 TAG Reconcile #759

anirudhAgniRedhat · 2024-09-19T09:03:23Z

This piece of code adds the support to append the extra tags provided by the user for DAY2 Implementation.
The background for this requirement can be found in https://issues.redhat.com/browse/CFE-1122

codecov · 2024-09-19T11:17:51Z

Codecov Report

Attention: Patch coverage is 42.30769% with 45 lines in your changes missing coverage. Please review.

Project coverage is 47.15%. Comparing base (2c25ae6) to head (3ab951a).
Report is 12 commits behind head on master.

Files with missing lines	Patch %	Lines
...redentialsrequest/credentialsrequest_controller.go	13.72%	44 Missing ⚠️
pkg/aws/actuator/actuator.go	96.29%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #759      +/-   ##
==========================================
- Coverage   47.28%   47.15%   -0.14%     
==========================================
  Files          96       96              
  Lines       11712    11789      +77     
==========================================
+ Hits         5538     5559      +21     
- Misses       5563     5617      +54     
- Partials      611      613       +2

Flag	Coverage Δ
	`47.15% <42.30%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/assets/bootstrap/bindata.go	`23.85% <ø> (ø)`
pkg/aws/actuator/actuator.go	`64.65% <96.29%> (+0.59%)`	⬆️
...redentialsrequest/credentialsrequest_controller.go	`44.23% <13.72%> (-2.35%)`	⬇️

... and 2 files with indirect coverage changes

openshift-ci-robot · 2024-09-22T06:58:22Z

@anirudhAgniRedhat: This pull request references CFE-1130 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

Details

In response to this:

This piece of code adds the support to append the extra tags provided by the user for DAY2 Implementation.
The background for this requirement can be found in https://issues.redhat.com/browse/CFE-1122

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 openshift-eng/jira-lifecycle-plugin repository.

anirudhAgniRedhat · 2024-09-24T06:59:54Z

/retest

jstuever · 2024-09-24T16:18:14Z

/assign

anirudhAgniRedhat · 2024-09-25T06:32:24Z

/retest

anirudhAgniRedhat · 2024-09-26T12:40:54Z

Hey @jstuever Can you please provide me the review for the changes

Thanks!

jstuever

I apologize for the delay; the reordering and reformatting of the yaml files made this unnecessarily difficult to review. I had a few questions, nitpicks, notes, and requested changes.

manifests/00-config-custresdef.yaml

pkg/aws/actuator/actuator.go

pkg/operator/credentialsrequest/credentialsrequest_controller.go

anirudhAgniRedhat · 2024-09-30T08:36:22Z

/retest

anirudhAgniRedhat · 2024-09-30T08:42:39Z

I apologize for the delay; the reordering and reformatting of the yaml files made this unnecessarily difficult to review. I had a few questions, nitpicks, notes, and requested changes.

I Agree!! but updating the API vendor was necessary here!! Thanks for the initial reviews, I have added the requested changes!!

jstuever · 2024-09-30T17:24:37Z

/test e2e-hypershift

jstuever · 2024-09-30T17:25:48Z

/hold
Ensure e2e-hypershift passes due to adding a new watch() and prior hypershift issues with memory usage.

jstuever · 2024-09-30T17:26:02Z

/lgtm
/approve

openshift-ci · 2024-09-30T17:26:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anirudhAgniRedhat, jstuever

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [jstuever]

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

jstuever · 2024-09-30T18:10:13Z

/test e2e-hypershift

anirudhAgniRedhat · 2024-09-30T19:44:07Z

/retest
Error message states

  * could not run steps: step e2e-hypershift failed: "e2e-hypershift" pre steps failed: could not determine image pull spec for image hypershift-operator on step hypershift-install 
INFO[2024-09-30T18:13:17Z] Reporting job state 'failed' with reason 'executing_graph:step_failed:utilizing_lease:executing_test:executing_multi_stage_test'

Need to check a bit on this!! previously these were passing for me before the latest commit which I don't feel should be a breaking change!

anirudhAgniRedhat · 2024-10-01T20:02:52Z

/retest

openshift-ci · 2024-10-01T21:46:04Z

@anirudhAgniRedhat: all tests passed!

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

jstuever · 2024-10-02T17:07:09Z

/hold cancel

openshift-bot · 2024-10-02T21:50:55Z

[ART PR BUILD NOTIFIER]

Distgit: ose-cloud-credential-operator
This PR has been included in build ose-cloud-credential-operator-container-v4.18.0-202410022110.p0.ge133084.assembly.stream.el9.
All builds following this will include this PR.

…condition Adding a "NOT" to the logged cloudCredsSecretUpdated field, because the following 'if' condition is !cloudCredsSecretUpdated. The lack of "NOT" seems to have been accidental oversight when the logging fields were added in 0a0d849 (Changes to address PR comments from Steve ~3d ago, 2023-06-27, openshift#542). I'm also adding isInfrastructureUpdated logging to catch up with cea55c6 (Added implementation for AWS Day2 Tag reconcilation Support, 2024-09-24, openshift#759), when it was added to the 'if' condition but overlooked in field logging.

…ncileUpdate CFE-1130: AWS DAY2 TAG Reconcile

Bumped o/api and updated manifests

c21d871

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 19, 2024

openshift-ci bot requested review from 2uasimojo and lleshchi September 19, 2024 09:03

anirudhAgniRedhat force-pushed the AWS_DAY2_reconcileUpdate branch from 510f686 to 338c63a Compare September 20, 2024 05:08

anirudhAgniRedhat changed the title ~~[WIP] AWS DAY2 TAG Reconcile~~ [WIP] CFE-1130: AWS DAY2 TAG Reconcile Sep 22, 2024

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

anirudhAgniRedhat force-pushed the AWS_DAY2_reconcileUpdate branch from 338c63a to 38d2f14 Compare September 23, 2024 20:40

anirudhAgniRedhat changed the title ~~[WIP] CFE-1130: AWS DAY2 TAG Reconcile~~ CFE-1130: AWS DAY2 TAG Reconcile Sep 24, 2024

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 Sep 24, 2024

openshift-ci bot assigned jstuever Sep 24, 2024

Added implementation for AWS Day2 Tag reconcilation Support

cea55c6

anirudhAgniRedhat force-pushed the AWS_DAY2_reconcileUpdate branch from 38d2f14 to cea55c6 Compare September 24, 2024 20:11

jstuever requested changes Sep 27, 2024

View reviewed changes

Added Review nits

3ab951a

anirudhAgniRedhat requested a review from jstuever September 30, 2024 05:15

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

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

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

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

openshift-merge-bot bot merged commit e133084 into openshift:master Oct 2, 2024

ming1013 pushed a commit to ming1013/cloud-credential-operator that referenced this pull request Dec 15, 2025

Merge pull request openshift#759 from anirudhAgniRedhat/AWS_DAY2_reco…

7f7f225

…ncileUpdate CFE-1130: AWS DAY2 TAG Reconcile

CFE-1130: AWS DAY2 TAG Reconcile #759

CFE-1130: AWS DAY2 TAG Reconcile #759

Uh oh!

Conversation

anirudhAgniRedhat commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

openshift-ci-robot commented Sep 22, 2024 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anirudhAgniRedhat commented Sep 24, 2024

Uh oh!

jstuever commented Sep 24, 2024

Uh oh!

anirudhAgniRedhat commented Sep 25, 2024

Uh oh!

anirudhAgniRedhat commented Sep 26, 2024

Uh oh!

jstuever left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anirudhAgniRedhat commented Sep 30, 2024

Uh oh!

anirudhAgniRedhat commented Sep 30, 2024

Uh oh!

jstuever commented Sep 30, 2024

Uh oh!

jstuever commented Sep 30, 2024

Uh oh!

jstuever commented Sep 30, 2024

Uh oh!

openshift-ci bot commented Sep 30, 2024

Uh oh!

jstuever commented Sep 30, 2024

Uh oh!

anirudhAgniRedhat commented Sep 30, 2024

Uh oh!

anirudhAgniRedhat commented Oct 1, 2024

Uh oh!

openshift-ci bot commented Oct 1, 2024

Uh oh!

jstuever commented Oct 2, 2024

Uh oh!

openshift-bot commented Oct 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anirudhAgniRedhat commented Sep 19, 2024 •

edited

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading

openshift-ci-robot commented Sep 22, 2024 •

edited by openshift-ci bot

Loading