Skip to content

Conversation

@jaypipes
Copy link
Collaborator

This is the first patch that looks at cleaning up our handling of
Condition objects for resources. We make a change to the common runtime
to clear all Condition objects from the desired resource's
Status.Conditions collection at the beginning of the reconciliation loop
and add a deferred call to a function that ensures that an
ACK.ResourceSynced condition has been placed on the resource if the
resource manager has failed to add one.

Future patches will add functionality to the code generator to allow
programmatic control over when generated resource managers set
ACK.ResourceSynced conditions on their resources.

Issue: aws-controllers-k8s/community#1030

Signed-off-by: Jay Pipes jaypipes@gmail.com

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

This is the first patch that looks at cleaning up our handling of
Condition objects for resources. We make a change to the common runtime
to clear all Condition objects from the desired resource's
Status.Conditions collection at the beginning of the reconciliation loop
and add a deferred call to a function that ensures that an
ACK.ResourceSynced condition has been placed on the resource if the
resource manager has failed to add one.

Future patches will add functionality to the code generator to allow
programmatic control over when generated resource managers set
ACK.ResourceSynced conditions on their resources.

Issue: aws-controllers-k8s/community#1030

Signed-off-by: Jay Pipes <jaypipes@gmail.com>
@vijtrip2
Copy link
Contributor

/approve

This looks good! But this still leaves individual resource managers to implement the ACK.ResourceSynced Status as True/False , which is currently missing.
And we should be aiming for that in future changes in code-generator. Correct?

@jaypipes
Copy link
Collaborator Author

/approve

This looks good! But this still leaves individual resource managers to implement the ACK.ResourceSynced Status as True/False , which is currently missing. And we should be aiming for that in future changes in code-generator. Correct?

Yup, exactly! see commit message :)

@vijtrip2
Copy link
Contributor

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2021
@ack-bot
Copy link
Collaborator

ack-bot commented Nov 11, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jaypipes, 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:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants