Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

make ScopeTemplate CRs communicate state #20

Merged
merged 27 commits into from
Oct 11, 2022
Merged

make ScopeTemplate CRs communicate state #20

merged 27 commits into from
Oct 11, 2022

Conversation

everettraven
Copy link
Collaborator

@everettraven everettraven commented Sep 7, 2022 •
edited
Loading

Description of the Change

  • Updates the ScopeTemplateStatus object to have a Conditions field that is of type []metav1.Conditions
  • Updates controllers/scopetemplate_controller.go to set a metav1.Condition with type Succeeded and updates the status of the condition accordingly
  • Updates controllers/scopetemplate_controller_test.go to:
    • Verify that the proper condition is being set
    • Create a unique ClusterRole for each iteration of a ScopeTemplate being created for the testing.
      • Before this, there was a clash in the tests in that the ClusterRole already existed even though the ScopeTemplate was being deleted and recreated. I think this raises the question: Should ClusterRoles be deleted if the ScopeTemplate that it is associated with is deleted?

Motivation for the Change

Fixes #4

@everettraven
Copy link
Collaborator Author

Once #21 is approved I will take what I learned from that PR and apply it to this one.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 14, 2022
@everettraven
Copy link
Collaborator Author

/hold

@openshift-ci 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 15, 2022
@everettraven
update scopeinstance_controller.go
add8cc0 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
update list options for delete in scopeinstance controller
4a9f89e 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
update controllers/scopetemplate_controller.go
18f3710 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
remove unecessary lines
1063a29 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
ignore not found error
9832f87 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
updates based on PR review
e99a979 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
update patch config
f80fcf2 
to just return unstructured.Unstructured instead of dealing with applyconfigurations

Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
Merge branch 'main' into chore/refactor-controllers
358d4b6
@everettraven
shared hash
19ff799 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
remove unnecessary comments
6eeb5d8 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
fix lint errors
8cbe23c 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
fix unit tests
ac6787c 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
update todo in mapToScopeTemplate
18514fd 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
make verify
a6a04ce 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
update hashing method in scopeinstance controller
7c180d6 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
first round of PR review fixes
0d806dd 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
second round of PR review updates
77b7c6f 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
updated missed log line to be debug level
5d6a4f9 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
update log message
b8d6c2d 
Signed-off-by: Bryce Palmer <[email protected]>
@everettraven
Merge branch 'main' into feature/scopetemplate-state
9db48c1
@everettraven
Merge branch 'chore/refactor-controllers' into feature/scopetemplate-…
d6082be 
…state
@everettraven
update method of setting scopetemplate status
aabe667 
after merging everettraven:chore/refactor-controllers

Signed-off-by: Bryce Palmer <[email protected]>
@openshift-merge-robot 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 Sep 28, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2022
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3182757679

  • 9 of 22 (40.91%) changed or added relevant lines in 1 file are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+1.06%) to 67.118%
Changes Missing Coverage Covered Lines Changed/Added Lines %
controllers/scopetemplate_controller.go 9 22 40.91%
Files with Coverage Reduction New Missed Lines %
controllers/scopetemplate_controller.go 4 60.75%
Totals Coverage Status
Change from base Build 3174960596: 1.06%
Covered Lines: 347
Relevant Lines: 517
💛 - Coveralls

@everettraven
Copy link
Collaborator Author

/hold cancel

@openshift-ci 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 4, 2022
jmrodri
jmrodri approved these changes Oct 10, 2022
View reviewed changes
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 10, 2022
@grokspawn
Copy link
Contributor

/lgtm

@everettraven everettraven merged commit 9665f86 into operator-framework:main Oct 11, 2022
@everettraven everettraven deleted the feature/scopetemplate-state branch October 11, 2022 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelanford joelanford joelanford left review comments

@jmrodri jmrodri jmrodri approved these changes

Assignees

@jmrodri jmrodri

@grokspawn grokspawn

Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ScopeTemplate CRs communicate state
6 participants
@everettraven @coveralls @grokspawn @jmrodri @joelanford @openshift-merge-robot