Skip to content

Add Owns(Secret, Deployment and ClusterOperator) to be watched#40

Merged
openshift-merge-robot merged 5 commits intoopenshift:masterfrom
asalkeld:watch-owned-resources
Nov 5, 2020
Merged

Add Owns(Secret, Deployment and ClusterOperator) to be watched#40
openshift-merge-robot merged 5 commits intoopenshift:masterfrom
asalkeld:watch-owned-resources

Conversation

@asalkeld
Copy link
Contributor

@asalkeld asalkeld commented Oct 9, 2020

Main objective

Make sure that if owned resources are deleted or modified, the Reconcile is run to fix these resources.
Owned resources are

  • deployment
  • secrets
  • clusterOperator

Note to reviewers

This may make sense to review by commit.

This commit does the following

  • add Owns() on ClusterOperator
  • reworks how the ClusterOperator is created as it now needs a baremetalConfig (for the ownership)
  • uses bindata to use the exact copy of the clusterOperator CR in manifests (This was does as the code was missing the required annotation and it does not seem right have multiple places for the same entity).
  • It also improves how the initial status and ownership is applied. There was a case when the CO was created by the manifest, it would not have the same details as when created in code.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 9, 2020
@asalkeld asalkeld force-pushed the watch-owned-resources branch from 3c18e3a to 826ebf7 Compare October 26, 2020 00:25
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2020
@asalkeld asalkeld changed the title WIP Add Owns(Secret and Deployment) to be watched Add Owns(Secret and Deployment) to be watched Oct 26, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 26, 2020
@asalkeld asalkeld force-pushed the watch-owned-resources branch from 826ebf7 to 4626e7e Compare October 26, 2020 00:28
@asalkeld asalkeld force-pushed the watch-owned-resources branch from 4626e7e to e8938b9 Compare October 27, 2020 09:28
@asalkeld asalkeld requested a review from dhellmann October 27, 2020 09:29
@asalkeld asalkeld force-pushed the watch-owned-resources branch from e8938b9 to cef9126 Compare October 29, 2020 02:54
@asalkeld asalkeld changed the title Add Owns(Secret and Deployment) to be watched Add Owns(Secret, Deployment and ClusterOperator) to be watched Oct 29, 2020
@asalkeld asalkeld force-pushed the watch-owned-resources branch 3 times, most recently from de12be4 to e54418a Compare November 2, 2020 00:02
Copy link
Contributor

@sadasu sadasu left a comment

Choose a reason for hiding this comment

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

This is clever. I have a couple of minor questions inline.

- Don't duplicate the definition of the clusterOperator
  note if created in code there is already a divergence (missing annotation)
- make the yaml manifest the master copy
- at each reconcile make sure ownership and status is correct
@asalkeld asalkeld force-pushed the watch-owned-resources branch from e54418a to b9ca309 Compare November 3, 2020 08:53
@asalkeld asalkeld requested a review from sadasu November 4, 2020 23:47
Copy link
Contributor

@sadasu sadasu left a comment

Choose a reason for hiding this comment

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

/lgtm
Thanks for taking care of my comments.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 5, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: asalkeld, sadasu

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

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants