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

STOR-1334: update storage operator to read featuregates from API on standalone OCP #368

Merged
merged 2 commits into from May 29, 2023
Merged

STOR-1334: update storage operator to read featuregates from API on standalone OCP #368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec5b6d1
update api and library-go to get featureset API
deads2k May 8, 2023
b8b52e1
update storage operator to read featuregates from API on standalone OCP
deads2k May 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
40 changes: 21 additions & 19 deletions go.mod
View file
Open in desktop
Expand Up @@ -6,21 +6,21 @@ require (
github.com/go-logr/logr v1.2.4 // indirect
github.com/google/go-cmp v0.5.9
github.com/google/gofuzz v1.2.0 // indirect
github.com/openshift/api v0.0.0-20230414095907-0540dde8186d
github.com/openshift/api v0.0.0-20230503133300-8bbcb7ca7183
github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d
github.com/openshift/client-go v0.0.0-20230120202327-72f107311084
github.com/openshift/library-go v0.0.0-20230130121854-13dc1e57ef79
github.com/openshift/client-go v0.0.0-20230503144108-75015d2347cb
github.com/openshift/library-go v0.0.0-20230508110756-9b7abe2c9cbf
github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.64.0
github.com/prometheus/client_golang v1.14.0
github.com/spf13/cobra v1.6.1
golang.org/x/net v0.9.0 // indirect
k8s.io/api v0.26.3
k8s.io/apiextensions-apiserver v0.26.3
k8s.io/apimachinery v0.26.3
k8s.io/api v0.27.1
k8s.io/apiextensions-apiserver v0.27.1
k8s.io/apimachinery v0.27.1
k8s.io/client-go v12.0.0+incompatible
k8s.io/component-base v0.26.3
k8s.io/component-base v0.27.1
k8s.io/klog/v2 v2.90.1
k8s.io/utils v0.0.0-20230308161112-d77c459e9343 // indirect
k8s.io/utils v0.0.0-20230406110748-d93618cff8a2 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
)
Expand All @@ -30,12 +30,13 @@ require github.com/prometheus-operator/prometheus-operator/pkg/client v0.64.0
require (
github.com/NYTimes/gziphandler v1.1.1 // indirect
github.com/antlr/antlr4/runtime/Go/antlr v1.4.10 // indirect
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/blang/semver/v4 v4.0.0 // indirect
github.com/cenkalti/backoff/v4 v4.2.0 // indirect
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/coreos/go-semver v0.3.0 // indirect
github.com/coreos/go-systemd/v22 v22.3.2 // indirect
github.com/coreos/go-systemd/v22 v22.4.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/emicklei/go-restful/v3 v3.10.1 // indirect
github.com/evanphx/json-patch v5.6.0+incompatible // indirect
Expand All @@ -60,6 +61,7 @@ require (
github.com/json-iterator/go v1.1.12 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
github.com/mitchellh/mapstructure v1.4.1 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
Expand All @@ -70,12 +72,12 @@ require (
github.com/prometheus/common v0.42.0 // indirect
github.com/prometheus/procfs v0.9.0 // indirect
github.com/robfig/cron v1.2.0 // indirect
github.com/sirupsen/logrus v1.8.1 // indirect
github.com/sirupsen/logrus v1.9.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/stoewer/go-strcase v1.2.0 // indirect
go.etcd.io/etcd/api/v3 v3.5.5 // indirect
go.etcd.io/etcd/client/pkg/v3 v3.5.5 // indirect
go.etcd.io/etcd/client/v3 v3.5.5 // indirect
go.etcd.io/etcd/api/v3 v3.5.7 // indirect
go.etcd.io/etcd/client/pkg/v3 v3.5.7 // indirect
go.etcd.io/etcd/client/v3 v3.5.7 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.35.0 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.40.0 // indirect
go.opentelemetry.io/otel v1.14.0 // indirect
Expand Down Expand Up @@ -104,14 +106,14 @@ require (
gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/apiserver v0.26.3 // indirect
k8s.io/kms v0.26.3 // indirect
k8s.io/kube-aggregator v0.26.1 // indirect
k8s.io/kube-openapi v0.0.0-20230303024457-afdc3dddf62d // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.36 // indirect
k8s.io/apiserver v0.27.1 // indirect
k8s.io/kms v0.27.1 // indirect
k8s.io/kube-aggregator v0.27.1 // indirect
k8s.io/kube-openapi v0.0.0-20230308215209-15aac26d736a // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.1.1 // indirect
sigs.k8s.io/controller-runtime v0.14.5 // indirect
sigs.k8s.io/kube-storage-version-migrator v0.0.4 // indirect
sigs.k8s.io/yaml v1.3.0 // indirect
)

replace k8s.io/client-go => k8s.io/client-go v0.26.1
replace k8s.io/client-go => k8s.io/client-go v0.27.0
201 changes: 100 additions & 101 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/operator/csidriveroperator/csioperatorclient/types.go
View file
Open in desktop
Expand Up @@ -64,7 +64,7 @@ type CSIOperatorConfig struct {
// OLMOptions configuration of migration from OLM to CSO
OLMOptions *OLMOptions
// Run the CSI driver operator only when given FeatureGate is enabled
RequireFeatureGate string
RequireFeatureGate configv1.FeatureGateName
}

// OLMOptions contains information that is necessary to remove old CSI driver
Expand Down
37 changes: 35 additions & 2 deletions pkg/operator/csidriveroperator/driverstarter_hypershift.go
View file
Open in desktop
Expand Up @@ -3,6 +3,8 @@ package csidriveroperator
import (
"bytes"
"context"
"time"

configv1 "github.com/openshift/api/config/v1"
operatorapi "github.com/openshift/api/operator/v1"
openshiftv1 "github.com/openshift/client-go/config/listers/config/v1"
Expand All @@ -11,6 +13,7 @@ import (
"github.com/openshift/cluster-storage-operator/pkg/operator/csidriveroperator/csioperatorclient"
"github.com/openshift/library-go/pkg/controller/factory"
"github.com/openshift/library-go/pkg/controller/manager"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
"github.com/openshift/library-go/pkg/operator/events"
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
"github.com/openshift/library-go/pkg/operator/staticresourcecontroller"
Expand All @@ -19,7 +22,6 @@ import (
storagelister "k8s.io/client-go/listers/storage/v1"
"k8s.io/client-go/restmapper"
"k8s.io/klog/v2"
"time"
)

// This CSIDriverStarterController starts CSI driver controllers based on the
Expand Down Expand Up @@ -122,6 +124,7 @@ func (c *CSIDriverStarterControllerHyperShift) sync(ctx context.Context, syncCtx
if err != nil {
return err
}
featureGateAccessor := createOldIncompatibleFeatureGatesForHypershift(featureGate)

// Start controller managers for this platform
for i := range c.controllers {
Expand All @@ -137,7 +140,7 @@ func (c *CSIDriverStarterControllerHyperShift) sync(ctx context.Context, syncCtx
}

if !ctrl.running {
shouldRun, err := shouldRunController(ctrl.operatorConfig, infrastructure, featureGate, csiDriver)
shouldRun, err := shouldRunController(ctrl.operatorConfig, infrastructure, featureGateAccessor, csiDriver)
if err != nil {
return err
}
Expand Down Expand Up @@ -167,6 +170,36 @@ func (c *CSIDriverStarterControllerHyperShift) sync(ctx context.Context, syncCtx
return nil
}

// createOldIncompatibleFeatureGatesForHypershift takes a featuregate and uses 4.13 logic to stitch together the individual
// featuregates. Eventually hypershift will (hopefully) follow core OCP and produce a central list.
func createOldIncompatibleFeatureGatesForHypershift(fg *configv1.FeatureGate) featuregates.FeatureGate {
if fg.Spec.FeatureSet == "" {
return nil
}
if fg.Spec.FeatureSet == configv1.CustomNoUpgrade {
enabled, disabled := []configv1.FeatureGateName{}, []configv1.FeatureGateName{}
if fg.Spec.CustomNoUpgrade != nil {
for _, curr := range fg.Spec.CustomNoUpgrade.Enabled {
enabled = append(enabled, curr)
}
for _, curr := range fg.Spec.CustomNoUpgrade.Disabled {
disabled = append(disabled, curr)
}
}
return featuregates.NewFeatureGate(enabled, disabled)
}

enabled, disabled := []configv1.FeatureGateName{}, []configv1.FeatureGateName{}
gates := configv1.FeatureSets[fg.Spec.FeatureSet]
for _, curr := range gates.Enabled {
enabled = append(enabled, curr.FeatureGateAttributes.Name)
}
for _, curr := range gates.Disabled {
disabled = append(disabled, curr.FeatureGateAttributes.Name)
}
return featuregates.NewFeatureGate(enabled, disabled)
}

func (c *CSIDriverStarterControllerHyperShift) createCSIControllerManager(
cfg csioperatorclient.CSIOperatorConfig,
resyncInterval time.Duration) (manager.ControllerManager, RelatedObjectGetter) {
Expand Down
78 changes: 24 additions & 54 deletions pkg/operator/csidriveroperator/driverstarter_standalone.go
View file
Open in desktop
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/openshift/cluster-storage-operator/pkg/operatorclient"
"github.com/openshift/library-go/pkg/controller/factory"
"github.com/openshift/library-go/pkg/controller/manager"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
"github.com/openshift/library-go/pkg/operator/events"
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
"github.com/openshift/library-go/pkg/operator/staticresourcecontroller"
Expand All @@ -23,6 +24,7 @@ import (
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/sets"
storagelister "k8s.io/client-go/listers/storage/v1"
"k8s.io/client-go/restmapper"
"k8s.io/klog/v2"
Expand All @@ -45,15 +47,15 @@ var (
// ControllerManagers for the particular cloud. It produces following Conditions:
// CSIDriverStarterDegraded - error checking the Infrastructure
type CSIDriverStarterController struct {
operatorClient *operatorclient.OperatorClient
infraLister openshiftv1.InfrastructureLister
featureGateLister openshiftv1.FeatureGateLister
csiDriverLister storagelister.CSIDriverLister
restMapper *restmapper.DeferredDiscoveryRESTMapper
versionGetter status.VersionGetter
targetVersion string
eventRecorder events.Recorder
controllers []csiDriverControllerManager
operatorClient *operatorclient.OperatorClient
infraLister openshiftv1.InfrastructureLister
featureGates featuregates.FeatureGate
csiDriverLister storagelister.CSIDriverLister
restMapper *restmapper.DeferredDiscoveryRESTMapper
versionGetter status.VersionGetter
targetVersion string
eventRecorder events.Recorder
controllers []csiDriverControllerManager
}

type RelatedObjectGetter interface {
Expand All @@ -71,20 +73,21 @@ type csiDriverControllerManager struct {

func NewCSIDriverStarterController(
clients *csoclients.Clients,
featureGates featuregates.FeatureGate,
resyncInterval time.Duration,
versionGetter status.VersionGetter,
targetVersion string,
eventRecorder events.Recorder,
driverConfigs []csioperatorclient.CSIOperatorConfig) factory.Controller {
c := &CSIDriverStarterController{
operatorClient: clients.OperatorClient,
infraLister: clients.ConfigInformers.Config().V1().Infrastructures().Lister(),
featureGateLister: clients.ConfigInformers.Config().V1().FeatureGates().Lister(),
csiDriverLister: clients.KubeInformers.InformersFor("").Storage().V1().CSIDrivers().Lister(),
restMapper: clients.RestMapper,
versionGetter: versionGetter,
targetVersion: targetVersion,
eventRecorder: eventRecorder.WithComponentSuffix("CSIDriverStarter"),
operatorClient: clients.OperatorClient,
infraLister: clients.ConfigInformers.Config().V1().Infrastructures().Lister(),
featureGates: featureGates,
csiDriverLister: clients.KubeInformers.InformersFor("").Storage().V1().CSIDrivers().Lister(),
restMapper: clients.RestMapper,
versionGetter: versionGetter,
targetVersion: targetVersion,
eventRecorder: eventRecorder.WithComponentSuffix("CSIDriverStarter"),
}
relatedObjects = []configv1.ObjectReference{}

Expand Down Expand Up @@ -129,10 +132,6 @@ func (c *CSIDriverStarterController) sync(ctx context.Context, syncCtx factory.S
if err != nil {
return err
}
featureGate, err := c.featureGateLister.Get(featureGateConfigName)
if err != nil {
return err
}

// Start controller managers for this platform
for i := range c.controllers {
Expand All @@ -148,7 +147,7 @@ func (c *CSIDriverStarterController) sync(ctx context.Context, syncCtx factory.S
}

if !ctrl.running {
shouldRun, err := shouldRunController(ctrl.operatorConfig, infrastructure, featureGate, csiDriver)
shouldRun, err := shouldRunController(ctrl.operatorConfig, infrastructure, c.featureGates, csiDriver)
if err != nil {
return err
}
Expand Down Expand Up @@ -247,7 +246,7 @@ func RelatedObjectFunc() func() (isset bool, objs []configv1.ObjectReference) {
}

// shouldRunController returns true, if given CSI driver controller should run.
func shouldRunController(cfg csioperatorclient.CSIOperatorConfig, infrastructure *configv1.Infrastructure, fg *configv1.FeatureGate, csiDriver *storagev1.CSIDriver) (bool, error) {
func shouldRunController(cfg csioperatorclient.CSIOperatorConfig, infrastructure *configv1.Infrastructure, fg featuregates.FeatureGate, csiDriver *storagev1.CSIDriver) (bool, error) {
// Check the correct platform first, it will filter out most CSI driver operators
var platform configv1.PlatformType
if infrastructure.Status.PlatformStatus != nil {
Expand All @@ -269,7 +268,8 @@ func shouldRunController(cfg csioperatorclient.CSIOperatorConfig, infrastructure
return true, nil
}

if !featureGateEnabled(fg, cfg.RequireFeatureGate) {
knownFeatures := sets.New[configv1.FeatureGateName](fg.KnownFeatures()...)
Copy link
Contributor

@sjenning sjenning May 30, 2023 •
edited

Choose a reason for hiding this comment

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

This is panicing on the hypershift control plane and blocking ci release stream

E0530 00:05:42.329947       1 runtime.go:79] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference)
goroutine 820 [running]:
k8s.io/apimachinery/pkg/util/runtime.logPanic({0x26a62a0?, 0x475c5c0})
	k8s.io/apimachinery@v0.27.1/pkg/util/runtime/runtime.go:75 +0x99
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0xc000699408, 0x1, 0x2545ca0?})
	k8s.io/apimachinery@v0.27.1/pkg/util/runtime/runtime.go:49 +0x75
panic({0x26a62a0, 0x475c5c0})
	runtime/panic.go:884 +0x213
github.com/openshift/cluster-storage-operator/pkg/operator/csidriveroperator.shouldRunController({{0x2b15110, 0x1f}, {0x2ade262, 0x6}, {0x2ae8cf2, 0xc}, 0x0, {0x0, 0x0, 0x0}, ...}, ...)
	github.com/openshift/cluster-storage-operator/pkg/operator/csidriveroperator/driverstarter_standalone.go:271 +0x157

https://amd64.ocp.releases.ci.openshift.org/releasestream/4.14.0-0.ci/release/4.14.0-0.ci-2023-05-29-185629

hypershift conformance test failed on this PR catching the break, but the test is not blocking yet and it merged anyway

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_cluster-storage-operator/368/pull-ci-openshift-cluster-storage-operator-master-e2e-hypershift-ovn-conformance/1655648223684988928/artifacts/e2e-hypershift-ovn-conformance/dump/artifacts/hostedcluster-b82afc77cb6ba7dcaec8/cluster-scoped-resources/config.openshift.io/clusteroperators/storage.yaml

Because openshift/release#39783 has not yet merged

if !knownFeatures.Has(cfg.RequireFeatureGate) || !fg.Enabled(cfg.RequireFeatureGate) {
klog.V(4).Infof("Not starting %s: feature %s is not enabled", cfg.CSIDriverName, cfg.RequireFeatureGate)
return false, nil
}
Expand All @@ -284,36 +284,6 @@ func shouldRunController(cfg csioperatorclient.CSIOperatorConfig, infrastructure
return true, nil
}

// Get list of enabled feature fates from FeatureGate CR.
func getEnabledFeatures(fg *configv1.FeatureGate) []string {
if fg.Spec.FeatureSet == "" {
return nil
}
if fg.Spec.FeatureSet == configv1.CustomNoUpgrade {
if fg.Spec.CustomNoUpgrade != nil {
return fg.Spec.CustomNoUpgrade.Enabled
}
// User wants CustomNoUpgrade, but did not set any feature gate.
return nil
}
gates := configv1.FeatureSets[fg.Spec.FeatureSet]
if gates == nil {
return nil
}
return gates.Enabled
}

// featureGateEnabled returns true if a given feature is enabled in FeatureGate CR.
func featureGateEnabled(fg *configv1.FeatureGate, feature string) bool {
enabledFeatures := getEnabledFeatures(fg)
for _, f := range enabledFeatures {
if f == feature {
return true
}
}
return false
}

func isUnsupportedCSIDriverRunning(cfg csioperatorclient.CSIOperatorConfig, csiDriver *storagev1.CSIDriver) bool {
if csiDriver == nil {
return false
Expand Down