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

Bug 2012069: Refactoring Status controller #498

Merged
merged 25 commits into from
Oct 8, 2021
Merged

Bug 2012069: Refactoring Status controller #498

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
bec2914
refactor: simplifying the logic for merge method
Sep 13, 2021
28986b2
refactor: extracting methods
Sep 14, 2021
a32b4db
refactor: improving variable names
Sep 14, 2021
3cf6071
refactor: improves condition handling
Sep 14, 2021
f32abb3
refactor: rename status*.go to controller*.go
Sep 16, 2021
c6fd846
refactor: introduce controller status and status messages
Sep 16, 2021
2814df4
style: fixing linting issues
Sep 16, 2021
c4755bb
fix: error handling to status.go
Sep 16, 2021
5ee0837
style: converting TODOS to issues
Sep 17, 2021
cc26439
refactor: smaller fixes
Sep 17, 2021
fe535b4
fix: fixing allReady inverted logic.
Sep 20, 2021
1488b79
refactor: conditions entries to clusteroperator
Sep 21, 2021
9a45d22
fix: fixing some code issues
Sep 22, 2021
cbb7125
test: improve test coverage
Sep 22, 2021
0b6f95d
style: disable funlen for Test_conditions_setCondition
Sep 22, 2021
ab0bb88
refactor: replace switch with if
Sep 22, 2021
2adc54a
style: happy lint
Sep 22, 2021
262453a
refactor: remove useless operator status
Sep 23, 2021
105195d
chore: better test name
Sep 23, 2021
847635a
refactor: improve log msg and logic
Sep 23, 2021
e99b571
refactor: remove duplicated message
Sep 29, 2021
069fff9
chore: fixing broken test
Sep 29, 2021
65300b4
fix: reset status controller every time (testing)
Oct 4, 2021
53b5d39
fix: check healthy operator status
Oct 5, 2021
69a4731
fix: improving code based on review
Oct 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/controller/status/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ func (c *Controller) merge(clusterOperator *configv1.ClusterOperator) *configv1.
clusterOperator = newClusterOperator(c.name, nil)
}

// make sure to start a clean status controller
c.ctrlStatus.reset()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tremes So, after a few tests (and fighting with the e2e tests), it seems that this is the best solution. We keep an instance to the status controller into the controller (controller everywhere) and then it encapsulates the logic to handle the statuses. But, it needs to be reset every time that the merge is executed, otherwise, the operator status would be wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes I am looking at it and it seem we are reseting everything (also the line https://github.com/openshift/insights-operator/pull/498/files#diff-10c33b0f428af9470aef2747e42fc97d97150d338b151e836d9cdebbf4fbef42R149) eveyrtime....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeh, but in this case it is correct. We are "resetting" 'cause the merge method receives the clusterOperator when it is called.


// calculate the current controller state
allReady, lastTransition := c.currentControllerStatus()

Expand Down
4 changes: 4 additions & 0 deletions pkg/controller/status/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,7 @@ func (c *controllerStatus) hasStatus(id string) bool {
_, ok := c.statusMap[id]
return ok
}

func (c *controllerStatus) reset() {
c.statusMap = make(map[string]statusMessage)
}