OCPBUGS-11486: fix: introduce correct SNO leader election for openshift

[jakobmoellerdev]

This PR is huge since for correct leader election in an openshift-compliant manner, we will need to introduce https://github.com/openshift/library-go/

Of course since we vendor, this would be a big change.

For the leader election, I consumed https://github.com/openshift/library-go/blob/release-4.14/pkg/config/leaderelection/leaderelection.go#L153 and switched to a custom lease lock that is consuming the Openshift Leaderelection based on leases.

I also had to introduce a redirect of klog to our standard logger as the library uses klog and we didn't have a redirect setup in place. Now all klog messages get parsed into JSON correctly as intended (previously also throttling messages would be malformatted, this has been corrected with that same redirect)

P.S.: to completely fix OCPBUGS-11486 we will also need an upstream PR in topolvm which exposes new flags for all necessary leaderelection parameters in https://github.com/topolvm/topolvm/blob/d242dcc522ae3f4bc3fe692054205b1076319396/pkg/topolvm-controller/cmd/root.go#L47 If we all agree on that approach I will open up a PR there as well.

Results in the following logs:

{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"Watching namespace","Namespace":"openshift-storage"}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"The leader election gives 4 retries and allows for 30s of clock skew. The kube-apiserver downtime tolerance is 78s. Worst non-graceful lease acquisition is 2m43s. Worst graceful lease acquisition is {26s}.\n"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"Overwriting defaults with SNO leader election config as only a single node was discovered","node":"crc-ccssl-master-0"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"leader election config setup succeeded","retryPeriod":"1m0s","leaseDuration":"4m30s","renewDeadline":"4m0s","election-namespace":"openshift-storage","election-name":"1136b8a6.topolvm.io","disable":false}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.metrics","msg":"Metrics server is starting to listen","addr":"127.0.0.1:8080"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.builder","msg":"skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called","GVK":"lvm.topolvm.io/v1alpha1, Kind=LVMCluster"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.builder","msg":"Registering a validating webhook","GVK":"lvm.topolvm.io/v1alpha1, Kind=LVMCluster","path":"/validate-lvm-topolvm-io-v1alpha1-lvmcluster"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.webhook","msg":"Registering webhook","path":"/validate-lvm-topolvm-io-v1alpha1-lvmcluster"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"starting manager"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.webhook.webhooks","msg":"Starting webhook server"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.certwatcher","msg":"Updated current TLS certificate"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.webhook","msg":"Serving webhook server","host":"","port":9443}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"Starting server","kind":"health probe","addr":":8081"}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"starting server","path":"/metrics","kind":"metrics","addr":"127.0.0.1:8080"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.certwatcher","msg":"Starting certificate watcher"}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"attempting to acquire leader lease openshift-storage/1136b8a6.topolvm.io...\n"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"successfully acquired lease openshift-storage/1136b8a6.topolvm.io\n"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"Event(v1.ObjectReference{Kind:\"Lease\", Namespace:\"openshift-storage\", Name:\"1136b8a6.topolvm.io\", UID:\"147ce12f-fda4-4df3-833d-a9dde8037ba0\", APIVersion:\"coordination.k8s.io/v1\", ResourceVersion:\"90435\", FieldPath:\"\"}): type: 'Normal' reason: 'LeaderElection' lvms-operator-c85c9f6b9-2cm8w_44c63ecb-1338-4d92-9ce3-9cd370eaab8e became leader\n"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"Starting EventSource","controller":"persistentvolumeclaim","controllerGroup":"","controllerKind":"PersistentVolumeClaim","source":"kind source: *v1.PersistentVolumeClaim"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"Starting Controller","controller":"persistentvolumeclaim","controllerGroup":"","controllerKind":"PersistentVolumeClaim"}
...

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[openshift-ci-robot]

@jakobmoellerdev: This pull request references Jira Issue OCPBUGS-11486, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @radeore

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

In response to this:

This PR is huge since for correct leader election in an openshift-compliant manner, we will need to introduce https://github.com/openshift/library-go/

However this version has a release for 4.14 that is already fully upgrading all client apis of k8s to 1.27.x (which is good since k8s upgrades to 1.27.x in 4.14).

This lead to me also upgrading all our client libraries to 1.27.x, however "controller-runtime" just recently started upgrading upstream with their latest release of https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.15.0

This in turn lead to small API changes in the controller webhook and Watch signatures, but overall nothing crazy changed.

Of course since we vendor, this would be a big change.

For the leader election, I consumed https://github.com/openshift/library-go/blob/release-4.14/pkg/config/leaderelection/leaderelection.go#L153 and switched to a custom lease lock that is consuming the Openshift Leaderelection based on leases.

Quite the big change but opening draft here for discussion.

P.S.: to completely fix OCPBUGS-11486 we will also need an upstream PR in topolvm which exposes new flags for all necessary leaderelection parameters in https://github.com/topolvm/topolvm/blob/d242dcc522ae3f4bc3fe692054205b1076319396/pkg/topolvm-controller/cmd/root.go#L47 If we all agree on that approach I will open up a PR there as well

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.

[jakobmoellerdev]

/test all

[codecov-commenter]

Codecov Report

Merging #370 (08eb519) into main (a962b90) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #370   +/-   ##
=======================================
  Coverage   16.59%   16.59%           
=======================================
  Files          24       24           
  Lines        2061     2061           
=======================================
  Hits          342      342           
  Misses       1693     1693           
  Partials       26       26           

[openshift-ci-robot]

@jakobmoellerdev: This pull request references Jira Issue OCPBUGS-11486, which is valid.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @radeore

In response to this:

This PR is huge since for correct leader election in an openshift-compliant manner, we will need to introduce https://github.com/openshift/library-go/

However this version has a release for 4.14 that is already fully upgrading all client apis of k8s to 1.27.x (which is good since k8s upgrades to 1.27.x in 4.14).

This lead to me also upgrading all our client libraries to 1.27.x, however "controller-runtime" just recently started upgrading upstream with their latest release of https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.15.0

All these upgrades also lead to an upgrade of go min version to 1.20 so this had to be upgraded as well.

This in turn lead to small API changes in the controller webhook and Watch signatures, but overall nothing crazy changed.

Of course since we vendor, this would be a big change.

For the leader election, I consumed https://github.com/openshift/library-go/blob/release-4.14/pkg/config/leaderelection/leaderelection.go#L153 and switched to a custom lease lock that is consuming the Openshift Leaderelection based on leases.

Quite the big change but opening draft here for discussion.

P.S.: to completely fix OCPBUGS-11486 we will also need an upstream PR in topolvm which exposes new flags for all necessary leaderelection parameters in https://github.com/topolvm/topolvm/blob/d242dcc522ae3f4bc3fe692054205b1076319396/pkg/topolvm-controller/cmd/root.go#L47 If we all agree on that approach I will open up a PR there as well

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.

[jakobmoellerdev]

/test all

[jakobmoellerdev]

once #371 is in will rebase and only do SNO changes here

[jeff-roche]

/retest

[openshift-ci-robot]

@jakobmoellerdev: This pull request references Jira Issue OCPBUGS-11486, which is valid.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @radeore

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

In response to this:

This PR is huge since for correct leader election in an openshift-compliant manner, we will need to introduce https://github.com/openshift/library-go/

Of course since we vendor, this would be a big change.

For the leader election, I consumed https://github.com/openshift/library-go/blob/release-4.14/pkg/config/leaderelection/leaderelection.go#L153 and switched to a custom lease lock that is consuming the Openshift Leaderelection based on leases.

Quite the big change but opening draft here for discussion.

P.S.: to completely fix OCPBUGS-11486 we will also need an upstream PR in topolvm which exposes new flags for all necessary leaderelection parameters in https://github.com/topolvm/topolvm/blob/d242dcc522ae3f4bc3fe692054205b1076319396/pkg/topolvm-controller/cmd/root.go#L47 If we all agree on that approach I will open up a PR there as well

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.

[jakobmoellerdev]

/test all

[openshift-ci-robot]

@jakobmoellerdev: This pull request references Jira Issue OCPBUGS-11486, which is valid.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @radeore

In response to this:

This PR is huge since for correct leader election in an openshift-compliant manner, we will need to introduce https://github.com/openshift/library-go/

Of course since we vendor, this would be a big change.

For the leader election, I consumed https://github.com/openshift/library-go/blob/release-4.14/pkg/config/leaderelection/leaderelection.go#L153 and switched to a custom lease lock that is consuming the Openshift Leaderelection based on leases.

I also had to introduce a redirect of klog to our standard logger as the library uses klog and we didn't have a redirect setup in place. Now all klog messages get parsed into JSON correctly as intended (previously also throttling messages would be malformatted, this has been corrected with that same redirect)

P.S.: to completely fix OCPBUGS-11486 we will also need an upstream PR in topolvm which exposes new flags for all necessary leaderelection parameters in https://github.com/topolvm/topolvm/blob/d242dcc522ae3f4bc3fe692054205b1076319396/pkg/topolvm-controller/cmd/root.go#L47 If we all agree on that approach I will open up a PR there as well.

Results in the following logs:

{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"Watching namespace","Namespace":"openshift-storage"}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"The leader election gives 4 retries and allows for 30s of clock skew. The kube-apiserver downtime tolerance is 78s. Worst non-graceful lease acquisition is 2m43s. Worst graceful lease acquisition is {26s}.\n"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"Overwriting defaults with SNO leader election config as only a single node was discovered","node":"crc-ccssl-master-0"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"leader election config setup succeeded","retryPeriod":"1m0s","leaseDuration":"4m30s","renewDeadline":"4m0s","election-namespace":"openshift-storage","election-name":"1136b8a6.topolvm.io","disable":false}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.metrics","msg":"Metrics server is starting to listen","addr":"127.0.0.1:8080"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.builder","msg":"skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called","GVK":"lvm.topolvm.io/v1alpha1, Kind=LVMCluster"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.builder","msg":"Registering a validating webhook","GVK":"lvm.topolvm.io/v1alpha1, Kind=LVMCluster","path":"/validate-lvm-topolvm-io-v1alpha1-lvmcluster"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.webhook","msg":"Registering webhook","path":"/validate-lvm-topolvm-io-v1alpha1-lvmcluster"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"starting manager"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.webhook.webhooks","msg":"Starting webhook server"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.certwatcher","msg":"Updated current TLS certificate"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.webhook","msg":"Serving webhook server","host":"","port":9443}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"Starting server","kind":"health probe","addr":":8081"}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"starting server","path":"/metrics","kind":"metrics","addr":"127.0.0.1:8080"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.certwatcher","msg":"Starting certificate watcher"}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"attempting to acquire leader lease openshift-storage/1136b8a6.topolvm.io...\n"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"successfully acquired lease openshift-storage/1136b8a6.topolvm.io\n"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"Event(v1.ObjectReference{Kind:\"Lease\", Namespace:\"openshift-storage\", Name:\"1136b8a6.topolvm.io\", UID:\"147ce12f-fda4-4df3-833d-a9dde8037ba0\", APIVersion:\"coordination.k8s.io/v1\", ResourceVersion:\"90435\", FieldPath:\"\"}): type: 'Normal' reason: 'LeaderElection' lvms-operator-c85c9f6b9-2cm8w_44c63ecb-1338-4d92-9ce3-9cd370eaab8e became leader\n"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"Starting EventSource","controller":"persistentvolumeclaim","controllerGroup":"","controllerKind":"PersistentVolumeClaim","source":"kind source: *v1.PersistentVolumeClaim"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"Starting Controller","controller":"persistentvolumeclaim","controllerGroup":"","controllerKind":"PersistentVolumeClaim"}
...

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.

[jeff-roche]

/override ci/prow/lvm-operator-bundle-e2e-aws
/override ci/prow/ci-index-lvm-operator-static-deps

[openshift-ci]

@jeff-roche: Overrode contexts on behalf of jeff-roche: ci/prow/ci-index-lvm-operator-static-deps, ci/prow/lvm-operator-bundle-e2e-aws

In response to this:

/override ci/prow/lvm-operator-bundle-e2e-aws
/override ci/prow/ci-index-lvm-operator-static-deps

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]

@jakobmoellerdev: 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.

[jeff-roche]

/approve

[jeff-roche]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jakobmoellerdev, jeff-roche

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 [jeff-roche]

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

[openshift-ci-robot]

@jakobmoellerdev: Jira Issue OCPBUGS-11486: All pull requests linked via external trackers have merged:

• openshift/lvm-operator#370

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

In response to this:

This PR is huge since for correct leader election in an openshift-compliant manner, we will need to introduce https://github.com/openshift/library-go/

Of course since we vendor, this would be a big change.

For the leader election, I consumed https://github.com/openshift/library-go/blob/release-4.14/pkg/config/leaderelection/leaderelection.go#L153 and switched to a custom lease lock that is consuming the Openshift Leaderelection based on leases.

I also had to introduce a redirect of klog to our standard logger as the library uses klog and we didn't have a redirect setup in place. Now all klog messages get parsed into JSON correctly as intended (previously also throttling messages would be malformatted, this has been corrected with that same redirect)

P.S.: to completely fix OCPBUGS-11486 we will also need an upstream PR in topolvm which exposes new flags for all necessary leaderelection parameters in https://github.com/topolvm/topolvm/blob/d242dcc522ae3f4bc3fe692054205b1076319396/pkg/topolvm-controller/cmd/root.go#L47 If we all agree on that approach I will open up a PR there as well.

Results in the following logs:

{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"Watching namespace","Namespace":"openshift-storage"}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"The leader election gives 4 retries and allows for 30s of clock skew. The kube-apiserver downtime tolerance is 78s. Worst non-graceful lease acquisition is 2m43s. Worst graceful lease acquisition is {26s}.\n"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"Overwriting defaults with SNO leader election config as only a single node was discovered","node":"crc-ccssl-master-0"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"leader election config setup succeeded","retryPeriod":"1m0s","leaseDuration":"4m30s","renewDeadline":"4m0s","election-namespace":"openshift-storage","election-name":"1136b8a6.topolvm.io","disable":false}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.metrics","msg":"Metrics server is starting to listen","addr":"127.0.0.1:8080"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.builder","msg":"skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called","GVK":"lvm.topolvm.io/v1alpha1, Kind=LVMCluster"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.builder","msg":"Registering a validating webhook","GVK":"lvm.topolvm.io/v1alpha1, Kind=LVMCluster","path":"/validate-lvm-topolvm-io-v1alpha1-lvmcluster"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.webhook","msg":"Registering webhook","path":"/validate-lvm-topolvm-io-v1alpha1-lvmcluster"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"setup","msg":"starting manager"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.webhook.webhooks","msg":"Starting webhook server"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.certwatcher","msg":"Updated current TLS certificate"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.webhook","msg":"Serving webhook server","host":"","port":9443}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"Starting server","kind":"health probe","addr":":8081"}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"starting server","path":"/metrics","kind":"metrics","addr":"127.0.0.1:8080"}
{"level":"info","ts":"2023-07-28T12:45:51Z","logger":"controller-runtime.certwatcher","msg":"Starting certificate watcher"}
{"level":"info","ts":"2023-07-28T12:45:51Z","msg":"attempting to acquire leader lease openshift-storage/1136b8a6.topolvm.io...\n"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"successfully acquired lease openshift-storage/1136b8a6.topolvm.io\n"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"Event(v1.ObjectReference{Kind:\"Lease\", Namespace:\"openshift-storage\", Name:\"1136b8a6.topolvm.io\", UID:\"147ce12f-fda4-4df3-833d-a9dde8037ba0\", APIVersion:\"coordination.k8s.io/v1\", ResourceVersion:\"90435\", FieldPath:\"\"}): type: 'Normal' reason: 'LeaderElection' lvms-operator-c85c9f6b9-2cm8w_44c63ecb-1338-4d92-9ce3-9cd370eaab8e became leader\n"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"Starting EventSource","controller":"persistentvolumeclaim","controllerGroup":"","controllerKind":"PersistentVolumeClaim","source":"kind source: *v1.PersistentVolumeClaim"}
{"level":"info","ts":"2023-07-28T12:46:14Z","msg":"Starting Controller","controller":"persistentvolumeclaim","controllerGroup":"","controllerKind":"PersistentVolumeClaim"}
...

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.