Skip to content

fix: set status on acquistion - #4843

Merged
google-oss-prow[bot] merged 2 commits into
masterfrom
acpana/sqlinstance-fields-status
Jul 30, 2025
Merged

google-oss-prow[bot] merged 2 commits into
masterfrom
acpana/sqlinstance-fields-status

Conversation

@acpana

@acpana acpana commented Jul 23, 2025

Copy link
Copy Markdown
Collaborator

Change description

Fixes #4811

Special notes for your reviewer:

Does this PR add something which needs to be 'release noted'?

fix: set status on acquisition for SQLInstance controller
  • Reviewer reviewed release note.

Additional documentation e.g., references, usage docs, etc.:


Intended Milestone

Please indicate the intended milestone.

  • Reviewer tagged PR with the actual milestone.

Comment on lines +18 to +24
status:
conditions:
- lastTransitionTime: "1970-01-01T00:00:00Z"
message: The resource is up to date
reason: UpToDate
status: "True"
type: Ready

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

compare this with object 01, notice the lack of additional fields

@acpana acpana added this to the 1.134 milestone Jul 23, 2025
@acpana

acpana commented Jul 23, 2025

Copy link
Copy Markdown
Collaborator Author

I think the CI is broken at HEAD!

acpana added 2 commits July 28, 2025 19:45
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com>
@acpana
acpana force-pushed the acpana/sqlinstance-fields-status branch from 5026b03 to 7aa6a6d Compare July 28, 2025 19:46
settings:
tier: db-f1-micro
---
TEST: ABANDON

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why don't we use ABANDON-AND-REACQUIRE directly?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

the test needs a resourceID and I didn't want to introduce that to this yaml.

return err
}

instanceForStatus := a.actual

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: maybe use updated := a.actual so that we don't need this additional variable instanceForStatus.

for example:

updated := a.actual
if xxx {
...
updated, err = a.sqlInstancesClient.Get(a.projectID, a.resourceID).Context(ctx).Do()
...
}
status, err := SQLInstanceStatusGCPToKRM(updated)

@gemmahou

Copy link
Copy Markdown
Collaborator

/lgtm

@yuwenma yuwenma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/approve

One nit. Non blocker.

Comment on lines +6 to +7
annotations:
cnrm.cloud.google.com/deletion-policy: abandon

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: ABANDON sets the "deletion-policy: abandon" already.

setAnnotation(h, obj, "cnrm.cloud.google.com/deletion-policy", "abandon")

@google-oss-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yuwenma

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:

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

@google-oss-prow
google-oss-prow Bot merged commit 6b61135 into master Jul 30, 2025
@cheftako
cheftako deleted the acpana/sqlinstance-fields-status branch April 1, 2026 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQLInstance status incomplete when adopting existing resource

3 participants