Skip to content

Commit

Permalink
Merge pull request red-hat-storage#2374 from umangachapagain/refactor…
Browse files Browse the repository at this point in the history
…-platform-detection

Refactor platform detection
  • Loading branch information
openshift-merge-bot[bot] authored Jan 29, 2024
2 parents f64d6b2 + cc44b17 commit 66b7cb7
Show file tree
Hide file tree
Showing 250 changed files with 23,067 additions and 420 deletions.
162 changes: 162 additions & 0 deletions controllers/platform/platform_detection.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
package platform

import (
"context"
"errors"
"log"
"sync"

configv1 "github.com/openshift/api/config/v1"
secv1 "github.com/openshift/api/security/v1"
configv1Client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/discovery"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
)

var (
once sync.Once
platformInstance *platform

ErrorPlatformNotDetected = errors.New("platform not detected")

// SkipObjectStorePlatforms is a list of all PlatformTypes where CephObjectStores will not be deployed.
SkipObjectStorePlatforms = []configv1.PlatformType{
configv1.AWSPlatformType,
configv1.GCPPlatformType,
configv1.AzurePlatformType,
configv1.IBMCloudPlatformType,
}
)

// platform is used to get the PlatformType of the running cluster in a thread-safe manner
// It is a singleton which is initialized exactly once via Detect() function call.
type platform struct {
isOpenShift bool
platform configv1.PlatformType
}

// SetFakePlatformInstanceForTesting can be used to fake a Platform while testing.
// It should only be used for testing. This is not thread-safe.
func SetFakePlatformInstanceForTesting(isOpenShift bool, platformType configv1.PlatformType) {
platformInstance = &platform{
isOpenShift: isOpenShift,
platform: platformType,
}
}

// UnsetFakePlatformInstanceForTesting can be used to unset the fake Platform while testing.
// It should only be used for testing. This is not thread-safe.
func UnsetFakePlatformInstanceForTesting() {
platformInstance = &platform{}
}

// Detect instantiates the platform only once. It is thread-safe.
func Detect() {
if platformInstance != nil {
return
}
once.Do(func() {
platformInstance = &platform{}
cfg := ctrl.GetConfigOrDie()
dclient := discovery.NewDiscoveryClientForConfigOrDie(cfg)
_, apis, err := dclient.ServerGroupsAndResources()
if err != nil && !discovery.IsGroupDiscoveryFailedError(err) {
log.Fatal(err)
}

if discovery.IsGroupDiscoveryFailedError(err) {
e := err.(*discovery.ErrGroupDiscoveryFailed)
if _, exists := e.Groups[secv1.GroupVersion]; exists {
platformInstance.isOpenShift = true
}
}

if !platformInstance.isOpenShift {
for _, api := range apis {
if api.GroupVersion == secv1.GroupVersion.String() {
for _, resource := range api.APIResources {
if resource.Name == "securitycontextconstraints" {
platformInstance.isOpenShift = true
break
}
}
}
if platformInstance.isOpenShift {
break
}
}
}

if platformInstance.isOpenShift {
if infrastructure, err := configv1client(cfg).Infrastructures().Get(context.TODO(), "cluster", metav1.GetOptions{}); err != nil {
platformInstance.platform = infrastructure.Status.PlatformStatus.Type
}
}
})
}

func configv1client(cfg *rest.Config) *configv1Client.ConfigV1Client {
return configv1Client.NewForConfigOrDie(cfg)
}

// IsPlatformOpenShift returns true if platform is detected to be OpenShift.
// It returns false in all other cases, including when platform is not yet detected.
func IsPlatformOpenShift() (bool, error) {
if platformInstance == nil {
return false, ErrorPlatformNotDetected
}
return platformInstance.isOpenShift, nil
}

// GetPlatformType is used to get the PlatformType of the running cluster.
// It returns a PlatformType only when running on OpenShift clusters.
// If it is not running on OpenShift or platform is not yet detected, it return empty PlatformType.
func GetPlatformType() (configv1.PlatformType, error) {
if platformInstance == nil {
return "", ErrorPlatformNotDetected
}
return platformInstance.platform, nil
}

// DevicesDefaultToFastForThisPlatform determines whether we should TuneFastDeviceClass for this platform.
// It returns false if we shouldn't TuneFastDeviceClass or if platform is not yet detected.
func DevicesDefaultToFastForThisPlatform() (bool, error) {
// tuneFastPlatforms is a list of all PlatformTypes where TuneFastDeviceClass has to be set True.
var tuneFastPlatforms = []configv1.PlatformType{
configv1.OvirtPlatformType,
configv1.IBMCloudPlatformType,
configv1.AzurePlatformType,
}
platform, err := GetPlatformType()
if err != nil {
return false, err
}
for _, tfplatform := range tuneFastPlatforms {
if platform == tfplatform {
return true, nil
}
}

return false, nil
}

// PlatformsShouldSkipObjectStore determines whether an object store should be created
// for the platform. It returns false if ObjectStore should not be skipped or if platform is not yet detected.
func PlatformsShouldSkipObjectStore() (bool, error) {
platform, err := GetPlatformType()
if err != nil {
return false, err
}
return SkipObjectStore(platform), nil
}

func SkipObjectStore(p configv1.PlatformType) bool {
for _, platform := range SkipObjectStorePlatforms {
if p == platform {
return true
}
}
return false
}
21 changes: 12 additions & 9 deletions controllers/storagecluster/backingstorageclasses.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

configv1 "github.com/openshift/api/config/v1"
ocsv1 "github.com/red-hat-storage/ocs-operator/api/v4/v1"
"github.com/red-hat-storage/ocs-operator/v4/controllers/platform"
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
v1 "k8s.io/api/storage/v1"
Expand All @@ -26,12 +27,8 @@ type backingStorageClasses struct{}
// ensureCreated ensures that backing storageclasses are created for the StorageCluster
func (obj *backingStorageClasses) ensureCreated(r *StorageClusterReconciler, sc *ocsv1.StorageCluster) (reconcile.Result, error) {

platform, err := r.platform.getPlatform(r.Client)
if err != nil {
return reconcile.Result{}, err
}
existingBackingStorageClasses := &v1.StorageClassList{}
err = r.Client.List(
err := r.Client.List(
r.ctx,
existingBackingStorageClasses,
client.MatchingLabels(map[string]string{
Expand All @@ -50,7 +47,7 @@ func (obj *backingStorageClasses) ensureCreated(r *StorageClusterReconciler, sc

for i := range sc.Spec.BackingStorageClasses {
bsc := &sc.Spec.BackingStorageClasses[i]
err := createOrUpdateBackingStorageclass(r, bsc, platform)
err := createOrUpdateBackingStorageclass(r, bsc)
if err != nil {
r.Log.Error(err, "Failed to create or update StorageClass.", "StorageClass", bsc.Name)
continue
Expand All @@ -75,10 +72,16 @@ func (obj *backingStorageClasses) ensureCreated(r *StorageClusterReconciler, sc
return reconcile.Result{}, nil
}

func createOrUpdateBackingStorageclass(r *StorageClusterReconciler, bsc *ocsv1.BackingStorageClass, platform configv1.PlatformType) error {
func createOrUpdateBackingStorageclass(r *StorageClusterReconciler, bsc *ocsv1.BackingStorageClass) error {
if bsc.Name == "" {
return fmt.Errorf("backingStorageClass name is empty")
}

platformType, err := platform.GetPlatformType()
if err != nil {
return err
}

pvReclaimPolicy := corev1.PersistentVolumeReclaimDelete
allowVolumeExpansion := true
volumeBindingMode := v1.VolumeBindingWaitForFirstConsumer
Expand All @@ -98,8 +101,8 @@ func createOrUpdateBackingStorageclass(r *StorageClusterReconciler, bsc *ocsv1.B
}

if bsc.Provisioner == "" {
if platform != configv1.AWSPlatformType {
return fmt.Errorf("auto detection of provisioner is not supported for %s", platform)
if platformType != configv1.AWSPlatformType {
return fmt.Errorf("auto detection of provisioner is not supported for %s", platformType)
}
desiredStorageClass.Provisioner = "ebs.csi.aws.com"
}
Expand Down
58 changes: 26 additions & 32 deletions controllers/storagecluster/cephblockpools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,15 @@ func TestCephBlockPools(t *testing.T) {
createRuntimeObjects: false,
},
}
for _, eachPlatform := range allPlatforms {
cp := &Platform{platform: eachPlatform}
for _, c := range cases {
var objects []client.Object
t, reconciler, cr, request := initStorageClusterResourceCreateUpdateTestWithPlatform(
t, cp, objects, nil)
if c.createRuntimeObjects {
objects = createUpdateRuntimeObjects(t, reconciler) //nolint:staticcheck //no need to use objects as they update in runtime
}
assertCephBlockPools(t, reconciler, cr, request, false, false)
assertCephNFSBlockPool(t, reconciler, cr, request)

for _, c := range cases {
var objects []client.Object
t, reconciler, cr, request := initStorageClusterResourceCreateUpdateTest(t, objects, nil)
if c.createRuntimeObjects {
objects = createUpdateRuntimeObjects(t) //nolint:staticcheck //no need to use objects as they update in runtime
}
assertCephBlockPools(t, reconciler, cr, request, false, false)
assertCephNFSBlockPool(t, reconciler, cr, request)
}
}

Expand Down Expand Up @@ -86,24 +83,21 @@ func TestInjectingPeerTokenToCephBlockPool(t *testing.T) {
},
}

for _, eachPlatform := range allPlatforms {
cp := &Platform{platform: eachPlatform}
for _, c := range cases {
cr := getInitData(c.spec)
request := reconcile.Request{
NamespacedName: types.NamespacedName{
Name: "ocsinit",
Namespace: "",
},
}
reconciler := createReconcilerFromCustomResources(t, cp, cr)
_, err := reconciler.Reconcile(context.TODO(), request)
assert.NoError(t, err)
if c.label == "test-injecting-peer-token-to-cephblockpool" {
assertCephBlockPools(t, reconciler, cr, request, true, true)
} else {
assertCephBlockPools(t, reconciler, cr, request, true, false)
}
for _, c := range cases {
cr := getInitData(c.spec)
request := reconcile.Request{
NamespacedName: types.NamespacedName{
Name: "ocsinit",
Namespace: "",
},
}
reconciler := createReconcilerFromCustomResources(t, cr)
_, err := reconciler.Reconcile(context.TODO(), request)
assert.NoError(t, err)
if c.label == "test-injecting-peer-token-to-cephblockpool" {
assertCephBlockPools(t, reconciler, cr, request, true, true)
} else {
assertCephBlockPools(t, reconciler, cr, request, true, false)
}
}
}
Expand All @@ -116,9 +110,9 @@ func getInitData(customSpec *api.StorageClusterSpec) *api.StorageCluster {
return cr
}

func createReconcilerFromCustomResources(t *testing.T, platform *Platform, cr *api.StorageCluster) StorageClusterReconciler {
reconciler := createFakeInitializationStorageClusterReconcilerWithPlatform(
t, platform, &nbv1.NooBaa{})
func createReconcilerFromCustomResources(t *testing.T, cr *api.StorageCluster) StorageClusterReconciler {
reconciler := createFakeInitializationStorageClusterReconciler(
t, &nbv1.NooBaa{})

secret := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Expand Down
11 changes: 7 additions & 4 deletions controllers/storagecluster/cephcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
monitoringclient "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned"
ocsv1 "github.com/red-hat-storage/ocs-operator/api/v4/v1"
"github.com/red-hat-storage/ocs-operator/v4/controllers/defaults"
"github.com/red-hat-storage/ocs-operator/v4/controllers/platform"
statusutil "github.com/red-hat-storage/ocs-operator/v4/controllers/util"
rookCephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -208,10 +209,12 @@ func (obj *ocsCephCluster) ensureCreated(r *StorageClusterReconciler, sc *ocsv1.
return reconcile.Result{}, err
}

platform, err := r.platform.GetPlatform(r.Client)
platform, err := platform.GetPlatformType()
if err != nil {
r.Log.Error(err, "Failed to get Platform.", "Platform", platform)
} else if platform == v1.IBMCloudPlatformType {
return reconcile.Result{}, err
}

if platform == v1.IBMCloudPlatformType {
r.Log.Info("Increasing Mon failover timeout to 15m.", "Platform", platform)
cephCluster.Spec.HealthCheck.DaemonHealth.Monitor.Timeout = "15m"
}
Expand Down Expand Up @@ -1017,7 +1020,7 @@ func (r *StorageClusterReconciler) checkTuneStorageDevices(ds ocsv1.StorageDevic
return dt.speed, nil
}

tuneFastDevices, err := r.DevicesDefaultToFastForThisPlatform()
tuneFastDevices, err := platform.DevicesDefaultToFastForThisPlatform()
if err != nil {
return diskSpeedUnknown, err
}
Expand Down
10 changes: 5 additions & 5 deletions controllers/storagecluster/cephcluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
api "github.com/red-hat-storage/ocs-operator/api/v4/v1"
ocsv1 "github.com/red-hat-storage/ocs-operator/api/v4/v1"
"github.com/red-hat-storage/ocs-operator/v4/controllers/defaults"
"github.com/red-hat-storage/ocs-operator/v4/controllers/platform"
"github.com/red-hat-storage/ocs-operator/v4/controllers/util"
ocsutil "github.com/red-hat-storage/ocs-operator/v4/controllers/util"
cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
Expand Down Expand Up @@ -196,16 +197,13 @@ func TestCephClusterMonTimeout(t *testing.T) {

for _, c := range cases {
t.Logf("Case: %s\n", c.label)
platform.SetFakePlatformInstanceForTesting(true, c.platform)

sc := &api.StorageCluster{}
mockStorageCluster.DeepCopyInto(sc)
sc.Status.Images.Ceph = &api.ComponentImageStatus{}

reconciler := createFakeStorageClusterReconciler(t, mockCephCluster.DeepCopy(), networkConfig)

reconciler.platform = &Platform{
platform: c.platform,
}

var obj ocsCephCluster
_, err := obj.ensureCreated(&reconciler, sc)
assert.NilError(t, err)
Expand All @@ -219,6 +217,8 @@ func TestCephClusterMonTimeout(t *testing.T) {
} else {
assert.Equal(t, "", cc.Spec.HealthCheck.DaemonHealth.Monitor.Timeout)
}

platform.UnsetFakePlatformInstanceForTesting()
}
}

Expand Down
Loading

0 comments on commit 66b7cb7

Please sign in to comment.