OCPBUGS-29126: deploymentcontroller: use builder pattern #1679

bertinatto · 2024-02-16T14:01:14Z

This is specially useful for controllers that don't want certain conditions.

CC @openshift/storage

openshift-ci-robot · 2024-02-16T14:01:20Z

@bertinatto: This pull request references Jira Issue OCPBUGS-29126, which is invalid:

expected the bug to target the "4.16.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Also, allow to update status condition.

CC @openshift/storage

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 openshift-eng/jira-lifecycle-plugin repository.

bertinatto · 2024-02-16T14:01:38Z

/hold
for manual testing

dusk125 · 2024-04-11T16:13:58Z

pkg/operator/deploymentcontroller/deployment_controller.go

@@ -249,3 +309,12 @@ func isProgressing(deployment *appsv1.Deployment) (bool, string) {
 	}
 	return false, ""
 }
+
+func containsCondition(conditions []string, condition string) bool {


nit: could use slices.Contains from standard library

…easy

dusk125 · 2024-04-12T17:49:58Z

/lgtm

bertinatto · 2024-04-29T17:29:22Z

/hold cancel
tested at openshift/cluster-storage-operator#456

deads2k · 2024-04-29T17:53:49Z

pkg/operator/deploymentcontroller/deployment_controller.go


+func (c *DeploymentController) WithConditions(conditions ...string) *DeploymentController {


missing doc.

Added docs.

pkg/operator/deploymentcontroller/deployment_controller.go

deads2k · 2024-04-29T18:00:08Z

pkg/operator/deploymentcontroller/deployment_controller.go

-		resourcemerge.SetDeploymentGeneration(&newStatus.Generations, deployment)
-		return nil
+	// Set Progressing Condition
+	if slices.Contains(c.conditions, opv1.OperatorStatusTypeProgressing) {


old code always had these, why make them "sometimes"?

The idea was to have the possibility to make every condition optional. NewDeploymentController still always has this by default.

deads2k · 2024-05-03T15:37:20Z

pkg/operator/deploymentcontroller/deployment_controller.go


-	return factory.New().WithInformers(
+// ToController converts the DeploymentController into a factory.Controller.
+func (c *DeploymentController) ToController() factory.Controller {


This could return an error so people would knwo they broke it.

Nice! Done.

openshift-ci · 2024-05-03T18:03:30Z

@bertinatto: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

deads2k · 2024-05-06T14:30:38Z

/lgtm
/approve

openshift-ci · 2024-05-06T14:31:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bertinatto, deads2k, dusk125

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:

~~pkg/operator/OWNERS~~ [deads2k]

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

bertinatto · 2024-05-06T16:23:26Z

/jira refresh

openshift-ci-robot · 2024-05-06T16:23:31Z

@bertinatto: This pull request references Jira Issue OCPBUGS-29126, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.16.0) matches configured target version for branch (4.16.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (wduan@redhat.com), skipping review request.

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-05-06T16:27:59Z

@bertinatto: Jira Issue OCPBUGS-29126: Some pull requests linked via external trackers have merged:

The following pull requests linked via external trackers have not merged:

openshift/cluster-storage-operator#456 is open

These pull request must merge or be unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

Jira Issue OCPBUGS-29126 has not been moved to the MODIFIED state.

In response to this:

This is specially useful for controllers that don't want certain conditions.

CC @openshift/storage

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-merge-robot · 2024-05-09T13:22:43Z

Fix included in accepted release 4.16.0-0.nightly-2024-05-08-222442

openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Feb 16, 2024

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 16, 2024

openshift-ci bot requested review from hexfusion and mfojtik February 16, 2024 14:02

bertinatto force-pushed the deploymentcontroller-builder branch 2 times, most recently from bfc2f34 to 5fafaba Compare March 6, 2024 17:37

dusk125 reviewed Apr 11, 2024

View reviewed changes

bertinatto added 2 commits April 12, 2024 14:29

deploymentcontroller: use the builder pattern to make customizations …

9c2fac4

…easy

deploymentcontroller: make it possible to pick conditions

d881918

bertinatto force-pushed the deploymentcontroller-builder branch from 5fafaba to d881918 Compare April 12, 2024 17:34

openshift-ci bot assigned dusk125 Apr 12, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 12, 2024

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 29, 2024

deads2k reviewed Apr 29, 2024

View reviewed changes

pkg/operator/deploymentcontroller/deployment_controller.go Show resolved Hide resolved

deads2k reviewed Apr 29, 2024

View reviewed changes

deploymentcontroller: add docstrings to new builder methods

cccbca1

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 2, 2024

deads2k reviewed May 3, 2024

View reviewed changes

bertinatto force-pushed the deploymentcontroller-builder branch from 035581c to aa6f19d Compare May 3, 2024 17:44

deploymentcontroller: only accept valid conditions in WithConditions

a5b4f1b

bertinatto force-pushed the deploymentcontroller-builder branch from aa6f19d to a5b4f1b Compare May 3, 2024 17:51

openshift-ci bot assigned deads2k May 6, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 6, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 6, 2024

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels May 6, 2024

openshift-merge-bot bot merged commit 6301add into openshift:master May 6, 2024
4 checks passed

openshift-ci-robot mentioned this pull request May 7, 2024

OCPBUGS-29126: vsphere-problem-detector: don't set Available condition openshift/cluster-storage-operator#456

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-29126: deploymentcontroller: use builder pattern #1679

OCPBUGS-29126: deploymentcontroller: use builder pattern #1679

bertinatto commented Feb 16, 2024 •

edited

openshift-ci-robot commented Feb 16, 2024

bertinatto commented Feb 16, 2024

dusk125 Apr 11, 2024

bertinatto Apr 12, 2024

dusk125 commented Apr 12, 2024

bertinatto commented Apr 29, 2024

deads2k Apr 29, 2024

bertinatto May 2, 2024

deads2k Apr 29, 2024

bertinatto May 2, 2024

deads2k May 3, 2024

bertinatto May 3, 2024

openshift-ci bot commented May 3, 2024

deads2k commented May 6, 2024

openshift-ci bot commented May 6, 2024

bertinatto commented May 6, 2024

openshift-ci-robot commented May 6, 2024

openshift-ci-robot commented May 6, 2024

openshift-merge-robot commented May 9, 2024


		func (c DeploymentController) WithConditions(conditions ...string) DeploymentController {

OCPBUGS-29126: deploymentcontroller: use builder pattern #1679

OCPBUGS-29126: deploymentcontroller: use builder pattern #1679

Conversation

bertinatto commented Feb 16, 2024 • edited

openshift-ci-robot commented Feb 16, 2024

bertinatto commented Feb 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dusk125 commented Apr 12, 2024

bertinatto commented Apr 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented May 3, 2024

deads2k commented May 6, 2024

openshift-ci bot commented May 6, 2024

bertinatto commented May 6, 2024

openshift-ci-robot commented May 6, 2024

openshift-ci-robot commented May 6, 2024

openshift-merge-robot commented May 9, 2024

bertinatto commented Feb 16, 2024 •

edited