Add logic to set the operator's status in the NFD CR #88

ArangoGutierrez · 2021-09-30T16:04:20Z

This PR rebases on #84

ArangoGutierrez · 2021-09-30T19:53:59Z

Let's merge #89 first
/hold

marquiz

Yeah, I think this basically looks good 👍 After #89 is merged just rebase and squash and I think we should be good to go

ArangoGutierrez · 2021-09-30T20:21:12Z

Ready @marquiz
/hold cancel

ArangoGutierrez · 2021-09-30T20:51:20Z

/assign@marquiz

ArangoGutierrez · 2021-09-30T21:06:38Z

re-re-re-re based

marquiz

Hmm, still spotted some oddities. It was now much easier to review as all the unrelated stuff was cleaned away 😄

controllers/nodefeaturediscovery_status.go

controllers/nodefeaturediscovery_controller.go

controllers/nodefeaturediscovery_status.go

ArangoGutierrez · 2021-10-01T15:01:32Z

ummm I see failed to rebase, 1 sec

ArangoGutierrez · 2021-10-01T16:23:55Z

/assign @marquiz

controllers/nodefeaturediscovery_controller.go

controllers/nodefeaturediscovery_status.go

marquiz

Looks like you had reverted some of the earlier fixes with the latest push. Also a new bunch of small comments and suggestions. But, overall, I feel that we're getting there and this should be the last review round 😊

controllers/nodefeaturediscovery_status.go

controllers/nodefeaturediscovery_controller.go

controllers/nodefeaturediscovery_status.go

marquiz

nit: I still see a multitude of those stale empty lines around.

After fixing those, I'd suggest to squash the commits and lets get this merged

marquiz · 2021-10-05T17:17:21Z

controllers/nodefeaturediscovery_status.go

+// getServiceConditions gets the current status of a Service. If an error
+// occurs, this function returns the corresponding error message
+func (r *NodeFeatureDiscoveryReconciler) getServiceConditions(ctx context.Context) (Status, error) {
+


Still there

ArangoGutierrez · 2021-10-05T18:51:01Z

nit: I still see a multitude of those stale empty lines around.

After fixing those, I'd suggest to squash the commits and lets get this merged

Done

marquiz

You hadn't answered to the question about isUpgradeable, just marked as resolved 🧐

marquiz · 2021-10-05T18:57:29Z

controllers/nodefeaturediscovery_status.go

+
+	// Is the resource available, upgradable, etc.?
+	isAvailable   bool
+	isUpgradeable bool


k8s-ci-robot · 2021-10-05T18:58:10Z

@marquiz:

In response to this:

/woof

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/test-infra repository.

ArangoGutierrez · 2021-10-05T19:02:01Z

You hadn't answered to the question about isUpgradeable, just marked as resolved monocle_face

totally skipped that comment, sorry, fixing...

Add logic that sets the operator's status in the NFD CR based on whether one or more of NFD's resources is: degraded, progressing, upgradeable, or available. Also add a "applyComponents" function to simplify readability. Signed-off-by: Carlos Eduardo Arango Gutierrez <carangog@redhat.com> Co-Authored-by: Courtney Pacheco <cpacheco@redhat.com> Co-authored-by: Markus Lehtonen <markus.lehtonen@intel.com>

marquiz

I think it's time
/lgtm

k8s-ci-robot · 2021-10-05T19:52:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ArangoGutierrez, marquiz

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:

~~OWNERS~~ [ArangoGutierrez,marquiz]

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

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 30, 2021

k8s-ci-robot requested review from marquiz and zvonkok September 30, 2021 16:04

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 30, 2021

ArangoGutierrez mentioned this pull request Sep 30, 2021

Add logic to set the operator's status in the NFD CR #84

Closed

ArangoGutierrez force-pushed the devel/rebase_84 branch from ac847c4 to 3be6c52 Compare September 30, 2021 16:08

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2021

ArangoGutierrez force-pushed the devel/rebase_84 branch from 3be6c52 to 8f595de Compare September 30, 2021 16:10

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 30, 2021

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

marquiz reviewed Sep 30, 2021

View reviewed changes

ArangoGutierrez force-pushed the devel/rebase_84 branch from 8989fa7 to f115fba Compare September 30, 2021 20:17

ArangoGutierrez requested a review from marquiz September 30, 2021 20:17

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 30, 2021

ArangoGutierrez force-pushed the devel/rebase_84 branch from f115fba to 5dcc9f6 Compare September 30, 2021 20:23

ArangoGutierrez force-pushed the devel/rebase_84 branch from 5dcc9f6 to 84fa1b5 Compare September 30, 2021 21:06

marquiz requested changes Oct 1, 2021

View reviewed changes

marquiz reviewed Oct 1, 2021

View reviewed changes

controllers/nodefeaturediscovery_status.go Outdated Show resolved Hide resolved

ArangoGutierrez force-pushed the devel/rebase_84 branch 2 times, most recently from 437f8da to 6be18b1 Compare October 1, 2021 14:58

ArangoGutierrez requested a review from marquiz October 1, 2021 14:58

ArangoGutierrez force-pushed the devel/rebase_84 branch from 6be18b1 to a829c05 Compare October 1, 2021 15:49

ArangoGutierrez force-pushed the devel/rebase_84 branch from a829c05 to 6f12e13 Compare October 1, 2021 15:51

ArangoGutierrez added the hacktoberfest-accepted hacktoberfest participating issue label Oct 1, 2021

k8s-ci-robot assigned marquiz Oct 1, 2021

ArangoGutierrez mentioned this pull request Oct 1, 2021

Add finalizers #86

Closed

marquiz requested changes Oct 2, 2021

View reviewed changes

marquiz reviewed Oct 2, 2021

View reviewed changes

controllers/nodefeaturediscovery_status.go Outdated Show resolved Hide resolved

ArangoGutierrez force-pushed the devel/rebase_84 branch from 7ffd130 to 4712346 Compare October 4, 2021 20:43

ArangoGutierrez requested a review from marquiz October 4, 2021 20:44

marquiz requested changes Oct 5, 2021

View reviewed changes

ArangoGutierrez requested a review from marquiz October 5, 2021 13:50

ArangoGutierrez force-pushed the devel/rebase_84 branch from 0a401e7 to 0d696dc Compare October 5, 2021 13:50

marquiz requested changes Oct 5, 2021

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 5, 2021

ArangoGutierrez force-pushed the devel/rebase_84 branch from 0d696dc to 5bb7c81 Compare October 5, 2021 18:50

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 5, 2021

ArangoGutierrez requested a review from marquiz October 5, 2021 18:50

marquiz requested changes Oct 5, 2021

View reviewed changes

ArangoGutierrez force-pushed the devel/rebase_84 branch from 5bb7c81 to 9b746f6 Compare October 5, 2021 19:03

ArangoGutierrez requested a review from marquiz October 5, 2021 19:17

marquiz approved these changes Oct 5, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 5, 2021

k8s-ci-robot merged commit 4c65abb into kubernetes-sigs:master Oct 5, 2021

marquiz mentioned this pull request Oct 26, 2021

Release v0.3.0 #85

Closed

13 tasks

ArangoGutierrez deleted the devel/rebase_84 branch March 13, 2023 10:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add logic to set the operator's status in the NFD CR #88

Add logic to set the operator's status in the NFD CR #88

ArangoGutierrez commented Sep 30, 2021

ArangoGutierrez commented Sep 30, 2021

marquiz left a comment

ArangoGutierrez commented Sep 30, 2021

ArangoGutierrez commented Sep 30, 2021

ArangoGutierrez commented Sep 30, 2021

marquiz left a comment

ArangoGutierrez commented Oct 1, 2021

ArangoGutierrez commented Oct 1, 2021

marquiz left a comment

marquiz left a comment

marquiz Oct 5, 2021

ArangoGutierrez commented Oct 5, 2021

marquiz left a comment

marquiz Oct 5, 2021

k8s-ci-robot commented Oct 5, 2021

ArangoGutierrez commented Oct 5, 2021

marquiz left a comment

k8s-ci-robot commented Oct 5, 2021

Add logic to set the operator's status in the NFD CR #88

Add logic to set the operator's status in the NFD CR #88

Conversation

ArangoGutierrez commented Sep 30, 2021

ArangoGutierrez commented Sep 30, 2021

marquiz left a comment

Choose a reason for hiding this comment

ArangoGutierrez commented Sep 30, 2021

ArangoGutierrez commented Sep 30, 2021

ArangoGutierrez commented Sep 30, 2021

marquiz left a comment

Choose a reason for hiding this comment

ArangoGutierrez commented Oct 1, 2021

ArangoGutierrez commented Oct 1, 2021

marquiz left a comment

Choose a reason for hiding this comment

marquiz left a comment

Choose a reason for hiding this comment

marquiz Oct 5, 2021

Choose a reason for hiding this comment

ArangoGutierrez commented Oct 5, 2021

marquiz left a comment

Choose a reason for hiding this comment

marquiz Oct 5, 2021

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 5, 2021

ArangoGutierrez commented Oct 5, 2021

marquiz left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 5, 2021