bug 2039670: Skip deployment of PDBs on the SNO topology by ingvagabund · Pull Request #109 · openshift/cluster-csi-snapshot-controller-operator

ingvagabund · 2022-01-12T13:35:13Z

Builds on top of openshift/library-go#1246 and WithIsSNOCheck from openshift/library-go#1238.

openshift-ci · 2022-01-12T13:35:18Z

@ingvagabund: This pull request references Bugzilla bug 2039670, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @zhouying7780

Details

In response to this:

bug 2039670: Deploy PDB to prevent more than one operand replica going unavailable

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.

bertinatto

@ingvagabund I think we got this sorted out already. Please take a look: af5cba6

ingvagabund · 2022-01-12T14:08:14Z

@bertinatto of course you have /O\ . I will just update the code so the PDBs are created conditionally. To avoid installation on the SNO topology.

openshift-ci · 2022-01-12T14:10:51Z

@ingvagabund: An error was encountered querying GitHub for users with public email (yinzhou@redhat.com) for bug 2039670 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message.


non-200 OK status code: 403 Forbidden body: "{\n  \"documentation_url\": \"https://docs.github.com/en/free-pro-team@latest/rest/overview/resources-in-the-rest-api#secondary-rate-limits\",\n  \"message\": \"You have exceeded a secondary rate limit. Please wait a few minutes before you try again.\"\n}\n"

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

Details

In response to this:

bug 2039670: Skip deployment of PDBs on the SNO topology

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.

ingvagabund · 2022-01-12T15:35:31Z

/hold

ingvagabund · 2022-01-12T21:35:53Z

/hold cancel

ingvagabund · 2022-01-13T08:41:57Z

/hold

ingvagabund · 2022-01-19T13:19:02Z

Once #110 gets merged I will rebase this PR.

ingvagabund · 2022-01-25T08:54:19Z

/test all

jsafrane · 2022-01-25T12:42:43Z

pkg/operator/starter.go

+			isSNO, precheckSucceeded, err := guard.IsSNOCheckFnc(configInformers.Config().V1().Infrastructures())()
+			if err != nil {
+				klog.Errorf("IsSNOCheckFnc failed: %v", err)
+				return false
+			}
+			if !precheckSucceeded {
+				klog.V(4).Infof("IsSNOCheckFnc precheck did not succeed, skipping")
+				return false
+			}
+			return !isSNO


Can you create isSNO() out of this and use isSNO() / !isSNO() below?

Unfortunately, the create/delete conditionals are not opposite of each other. They both can be false when:

configInformers.Config().V1().Infrastructures()'s hasSynced is false

retrieval of the cluster infrastructure object failed.

…logy

ingvagabund · 2022-01-25T13:28:37Z

/hold cancel

jsafrane · 2022-01-25T14:43:12Z

/lgtm
/approve

openshift-ci · 2022-01-25T14:46:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ingvagabund, jsafrane

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:

~~OWNERS~~ [jsafrane]

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

openshift-ci · 2022-01-25T15:14:39Z

@ingvagabund: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

openshift-ci · 2022-01-25T15:25:08Z

@ingvagabund: All pull requests linked via external trackers have merged:

Bugzilla bug 2039670 has been moved to the MODIFIED state.

Details

In response to this:

bug 2039670: Skip deployment of PDBs on the SNO topology

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.

openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jan 12, 2022

openshift-ci bot requested review from bertinatto, gnufied and zhouying7780 January 12, 2022 13:35

bertinatto reviewed Jan 12, 2022

View reviewed changes

ingvagabund force-pushed the pdbs-for-operands branch from 677623e to 9341966 Compare January 12, 2022 14:07

ingvagabund changed the title ~~bug 2039670: Deploy PDB to prevent more than one operand replica going unavailable~~ bug 2039670: Skip deploymen of PDB on the SNO topology Jan 12, 2022

ingvagabund force-pushed the pdbs-for-operands branch from 9341966 to a9f8a0a Compare January 12, 2022 14:09

ingvagabund changed the title ~~bug 2039670: Skip deploymen of PDB on the SNO topology~~ bug 2039670: Skip deployment of PDBs on the SNO topology Jan 12, 2022

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

ingvagabund force-pushed the pdbs-for-operands branch from a9f8a0a to 518a38d Compare January 12, 2022 21:35

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

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

ingvagabund force-pushed the pdbs-for-operands branch from 518a38d to bbbbb8c Compare January 19, 2022 13:18

openshift-ci bot mentioned this pull request Jan 20, 2022

bug 2039670: Sync the library-go openshift/cluster-authentication-operator#538

Merged

jsafrane reviewed Jan 25, 2022

View reviewed changes

ingvagabund added 2 commits January 25, 2022 14:20

Create the PDB dynamically so it does not get created on the SNO topo…

c094345

…logy

go mod vendor

2e2fcc9

ingvagabund force-pushed the pdbs-for-operands branch from bbbbb8c to 2e2fcc9 Compare January 25, 2022 13:25

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

openshift-ci bot assigned jsafrane Jan 25, 2022

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 25, 2022

openshift-merge-robot merged commit 068ddb9 into openshift:master Jan 25, 2022

ingvagabund deleted the pdbs-for-operands branch January 25, 2022 15:21

Conversation

ingvagabund commented Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jan 12, 2022

Uh oh!

bertinatto left a comment

Choose a reason for hiding this comment

Uh oh!

ingvagabund commented Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jan 12, 2022

Uh oh!

ingvagabund commented Jan 12, 2022

Uh oh!

ingvagabund commented Jan 12, 2022

Uh oh!

ingvagabund commented Jan 13, 2022

Uh oh!

ingvagabund commented Jan 19, 2022

Uh oh!

ingvagabund commented Jan 25, 2022

Uh oh!

jsafrane Jan 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ingvagabund Jan 25, 2022

Choose a reason for hiding this comment

Uh oh!

ingvagabund commented Jan 25, 2022

Uh oh!

jsafrane commented Jan 25, 2022

Uh oh!

openshift-ci bot commented Jan 25, 2022

Uh oh!

openshift-ci bot commented Jan 25, 2022

Uh oh!

openshift-ci bot commented Jan 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

ingvagabund commented Jan 12, 2022 •

edited

Loading

ingvagabund commented Jan 12, 2022 •

edited

Loading

jsafrane Jan 25, 2022 •

edited

Loading