WIP: STOR-2523: hypershift: Add desired-version annotation to controller Deployments #457
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@mpatlasov: This pull request references STOR-2523 which is a valid jira issue. In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mpatlasov 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:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/retest-required |
3 similar comments
|
/retest-required |
|
/retest-required |
|
/retest-required |
|
/assign @jsafrane |
openshift-merge-robot
added
the
needs-rebase
New annotation `release.openshift.io/desired-version` will be used by CSI driver operators to compare the version from env var `OPERATOR_IMAGE_VERSION` with actual version of currently running operand.
mpatlasov
force-pushed
the
STOR-2523-Add-desired-version-annotation
branch
from
7576b2b to
05353f3
Compare
openshift-merge-robot
removed
the
needs-rebase
|
/retest-required |
|
/hold Current approach requests Deployment from the hook like this: It's not good. Let's try another approach -- new controller that would reconcile "release.openshift.io/version" annotation. |
openshift-ci
bot
added
the
do-not-merge/hold
mpatlasov
force-pushed
the
STOR-2523-Add-desired-version-annotation
branch
from
05353f3 to
812763e
Compare
| return nil | ||
| } | ||
|
|
||
| klog.Infof("VersionController: getting deployment %s in namespace %s ...", c.controllerDeploymentName, c.commonClient.ControlPlaneNamespace) |
There was a problem hiding this comment.
Reduce the log level or remove this log completely, I am not sure how useful it is.
There was a problem hiding this comment.
The other logs look useful, just lower the level to V(4) for debugging and V(2) for useful low frequency changes (like deployment updated with version %s below).
| defaultReSyncPeriod = 10 * time.Minute | ||
| ) | ||
|
|
||
| type VersionController struct { |
There was a problem hiding this comment.
Please add documentation what the controller does, how it's useful and what it expects from the Deployment (presence of desired-version annotation?)
| defaultReSyncPeriod = 10 * time.Minute | ||
| ) | ||
|
|
||
| type VersionController struct { |
There was a problem hiding this comment.
I would probably add HyperShift either in the controller or package name, as it's very HCP specific.
| defaultReSyncPeriod = 10 * time.Minute | ||
| ) | ||
|
|
||
| type VersionController struct { |
There was a problem hiding this comment.
Please add some unit tests + move it to library-go. We will vendor it to CSO, snapshot operator, drivers and who knows where else.
| var desiredVersion string | ||
| var actualVersion string | ||
| for k, v := range deployment.Annotations { | ||
| if k == "release.openshift.io/desired-version" { |
There was a problem hiding this comment.
the annotation names should be constants
New annotation `release.openshift.io/version` signals that rollout of Deployment is complete (see https://issues.redhat.com/browse/STOR-2523 for details). This commit implements new controller (VersionController) which reconciles version annotation if controller deplyment is done.
mpatlasov
force-pushed
the
STOR-2523-Add-desired-version-annotation
branch
from
812763e to
1289852
Compare
|
/hold |
mpatlasov
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@mpatlasov: The following tests failed, say
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-sigs/prow repository. I understand the commands that are listed here. |
4 participants
New annotation
release.openshift.io/desired-versionwill be used by CSI driver operators to compare the version from env varOPERATOR_IMAGE_VERSIONwith actual version of currently running operand.