Skip to content

Commit 63a4f18

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1315 from openshift-splat-team/refactor-fg-usage
SPLAT-1940: Refactored vsphere featuregate parameters
2 parents fcc27c3 + 281f7d4 commit 63a4f18

15 files changed

+700
-97
lines changed

‎cmd/machineset/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func main() {
186186
log.Fatal(err)
187187
}
188188

189-
machineValidator, err := mapiwebhooks.NewMachineValidator(mgr.GetClient())
189+
machineValidator, err := mapiwebhooks.NewMachineValidator(mgr.GetClient(), defaultMutableGate)
190190
if err != nil {
191191
log.Fatal(err)
192192
}
@@ -196,7 +196,7 @@ func main() {
196196
log.Fatal(err)
197197
}
198198

199-
machineSetValidator, err := mapiwebhooks.NewMachineSetValidator(mgr.GetClient())
199+
machineSetValidator, err := mapiwebhooks.NewMachineSetValidator(mgr.GetClient(), defaultMutableGate)
200200
if err != nil {
201201
log.Fatal(err)
202202
}

‎cmd/vsphere/main.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,12 @@ func main() {
165165

166166
// Initialize machine actuator.
167167
machineActuator := machine.NewActuator(machine.ActuatorParams{
168-
Client: mgr.GetClient(),
169-
APIReader: mgr.GetAPIReader(),
170-
EventRecorder: mgr.GetEventRecorderFor("vspherecontroller"),
171-
TaskIDCache: taskIDCache,
172-
StaticIPFeatureGateEnabled: staticIPFeatureGateEnabled,
173-
OpenshiftConfigNamespace: vsphere.OpenshiftConfigNamespace,
168+
Client: mgr.GetClient(),
169+
APIReader: mgr.GetAPIReader(),
170+
EventRecorder: mgr.GetEventRecorderFor("vspherecontroller"),
171+
TaskIDCache: taskIDCache,
172+
FeatureGates: defaultMutableGate,
173+
OpenshiftConfigNamespace: vsphere.OpenshiftConfigNamespace,
174174
})
175175

176176
if err := configv1.Install(mgr.GetScheme()); err != nil {

‎pkg/controller/vsphere/actuator.go

Lines changed: 44 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"fmt"
88
"time"
99

10+
"k8s.io/component-base/featuregate"
11+
1012
machinev1 "github.com/openshift/api/machine/v1beta1"
1113
machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
1214
corev1 "k8s.io/api/core/v1"
@@ -27,33 +29,33 @@ const (
2729

2830
// Actuator is responsible for performing machine reconciliation.
2931
type Actuator struct {
30-
client runtimeclient.Client
31-
apiReader runtimeclient.Reader
32-
eventRecorder record.EventRecorder
33-
TaskIDCache map[string]string
34-
StaticIPFeatureGateEnabled bool
35-
openshiftConfigNamespace string
32+
client runtimeclient.Client
33+
apiReader runtimeclient.Reader
34+
eventRecorder record.EventRecorder
35+
TaskIDCache map[string]string
36+
FeatureGates featuregate.MutableFeatureGate
37+
openshiftConfigNamespace string
3638
}
3739

3840
// ActuatorParams holds parameter information for Actuator.
3941
type ActuatorParams struct {
40-
Client runtimeclient.Client
41-
APIReader runtimeclient.Reader
42-
EventRecorder record.EventRecorder
43-
TaskIDCache map[string]string
44-
StaticIPFeatureGateEnabled bool
45-
OpenshiftConfigNamespace string
42+
Client runtimeclient.Client
43+
APIReader runtimeclient.Reader
44+
EventRecorder record.EventRecorder
45+
TaskIDCache map[string]string
46+
FeatureGates featuregate.MutableFeatureGate
47+
OpenshiftConfigNamespace string
4648
}
4749

4850
// NewActuator returns an actuator.
4951
func NewActuator(params ActuatorParams) *Actuator {
5052
return &Actuator{
51-
client: params.Client,
52-
apiReader: params.APIReader,
53-
eventRecorder: params.EventRecorder,
54-
TaskIDCache: params.TaskIDCache,
55-
StaticIPFeatureGateEnabled: params.StaticIPFeatureGateEnabled,
56-
openshiftConfigNamespace: params.OpenshiftConfigNamespace,
53+
client: params.Client,
54+
apiReader: params.APIReader,
55+
eventRecorder: params.EventRecorder,
56+
TaskIDCache: params.TaskIDCache,
57+
FeatureGates: params.FeatureGates,
58+
openshiftConfigNamespace: params.OpenshiftConfigNamespace,
5759
}
5860
}
5961

@@ -72,12 +74,12 @@ func (a *Actuator) Create(ctx context.Context, machine *machinev1.Machine) error
7274
klog.Infof("%s: actuator creating machine", machine.GetName())
7375

7476
scope, err := newMachineScope(machineScopeParams{
75-
Context: ctx,
76-
client: a.client,
77-
machine: machine,
78-
apiReader: a.apiReader,
79-
StaticIPFeatureGateEnabled: a.StaticIPFeatureGateEnabled,
80-
openshiftConfigNameSpace: a.openshiftConfigNamespace,
77+
Context: ctx,
78+
client: a.client,
79+
machine: machine,
80+
apiReader: a.apiReader,
81+
featureGates: a.FeatureGates,
82+
openshiftConfigNameSpace: a.openshiftConfigNamespace,
8183
})
8284
if err != nil {
8385
fmtErr := fmt.Errorf(scopeFailFmt, machine.GetName(), err)
@@ -116,12 +118,12 @@ func (a *Actuator) Create(ctx context.Context, machine *machinev1.Machine) error
116118
func (a *Actuator) Exists(ctx context.Context, machine *machinev1.Machine) (bool, error) {
117119
klog.Infof("%s: actuator checking if machine exists", machine.GetName())
118120
scope, err := newMachineScope(machineScopeParams{
119-
Context: ctx,
120-
client: a.client,
121-
machine: machine,
122-
apiReader: a.apiReader,
123-
StaticIPFeatureGateEnabled: a.StaticIPFeatureGateEnabled,
124-
openshiftConfigNameSpace: a.openshiftConfigNamespace,
121+
Context: ctx,
122+
client: a.client,
123+
machine: machine,
124+
apiReader: a.apiReader,
125+
featureGates: a.FeatureGates,
126+
openshiftConfigNameSpace: a.openshiftConfigNamespace,
125127
})
126128
if err != nil {
127129
return false, fmt.Errorf(scopeFailFmt, machine.GetName(), err)
@@ -135,12 +137,12 @@ func (a *Actuator) Update(ctx context.Context, machine *machinev1.Machine) error
135137
delete(a.TaskIDCache, machine.Name)
136138

137139
scope, err := newMachineScope(machineScopeParams{
138-
Context: ctx,
139-
client: a.client,
140-
machine: machine,
141-
apiReader: a.apiReader,
142-
StaticIPFeatureGateEnabled: a.StaticIPFeatureGateEnabled,
143-
openshiftConfigNameSpace: a.openshiftConfigNamespace,
140+
Context: ctx,
141+
client: a.client,
142+
machine: machine,
143+
apiReader: a.apiReader,
144+
featureGates: a.FeatureGates,
145+
openshiftConfigNameSpace: a.openshiftConfigNamespace,
144146
})
145147
if err != nil {
146148
fmtErr := fmt.Errorf(scopeFailFmt, machine.GetName(), err)
@@ -177,12 +179,12 @@ func (a *Actuator) Delete(ctx context.Context, machine *machinev1.Machine) error
177179
delete(a.TaskIDCache, machine.Name)
178180

179181
scope, err := newMachineScope(machineScopeParams{
180-
Context: ctx,
181-
client: a.client,
182-
machine: machine,
183-
apiReader: a.apiReader,
184-
StaticIPFeatureGateEnabled: a.StaticIPFeatureGateEnabled,
185-
openshiftConfigNameSpace: a.openshiftConfigNamespace,
182+
Context: ctx,
183+
client: a.client,
184+
machine: machine,
185+
apiReader: a.apiReader,
186+
featureGates: a.FeatureGates,
187+
openshiftConfigNameSpace: a.openshiftConfigNamespace,
186188
})
187189
if err != nil {
188190
fmtErr := fmt.Errorf(scopeFailFmt, machine.GetName(), err)

‎pkg/controller/vsphere/actuator_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"testing"
99
"time"
1010

11+
testutils "github.com/openshift/machine-api-operator/pkg/util/testing"
12+
1113
. "github.com/onsi/gomega"
1214
configv1 "github.com/openshift/api/config/v1"
1315
machinev1 "github.com/openshift/api/machine/v1beta1"
@@ -363,13 +365,19 @@ func TestMachineEvents(t *testing.T) {
363365
}
364366
gs.Eventually(getNode, timeout).Should(Succeed())
365367

368+
gate, err := testutils.NewDefaultMutableFeatureGate()
369+
if err != nil {
370+
t.Errorf("Unexpected error setting up feature gates: %v", err)
371+
}
372+
366373
taskIDCache := make(map[string]string)
367374
params := ActuatorParams{
368375
Client: k8sClient,
369376
EventRecorder: eventRecorder,
370377
APIReader: k8sClient,
371378
TaskIDCache: taskIDCache,
372379
OpenshiftConfigNamespace: openshiftConfigNamespaceForTest,
380+
FeatureGates: gate,
373381
}
374382

375383
actuator := NewActuator(params)

‎pkg/controller/vsphere/machine_scope.go

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"errors"
66
"fmt"
77

8+
"k8s.io/component-base/featuregate"
9+
810
machinev1 "github.com/openshift/api/machine/v1beta1"
911
machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
1012
"github.com/openshift/machine-api-operator/pkg/controller/vsphere/session"
@@ -23,11 +25,11 @@ const (
2325
// machineScopeParams defines the input parameters used to create a new MachineScope.
2426
type machineScopeParams struct {
2527
context.Context
26-
client runtimeclient.Client
27-
apiReader runtimeclient.Reader
28-
machine *machinev1.Machine
29-
StaticIPFeatureGateEnabled bool
30-
openshiftConfigNameSpace string
28+
client runtimeclient.Client
29+
apiReader runtimeclient.Reader
30+
machine *machinev1.Machine
31+
featureGates featuregate.MutableFeatureGate
32+
openshiftConfigNameSpace string
3133
}
3234

3335
// machineScope defines a scope defined around a machine and its cluster.
@@ -42,11 +44,11 @@ type machineScope struct {
4244
// vSphere cloud-provider config
4345
vSphereConfig *vsphere.Config
4446
// machine resource
45-
machine *machinev1.Machine
46-
providerSpec *machinev1.VSphereMachineProviderSpec
47-
providerStatus *machinev1.VSphereMachineProviderStatus
48-
machineToBePatched runtimeclient.Patch
49-
staticIPFeatureGateEnabled bool
47+
machine *machinev1.Machine
48+
providerSpec *machinev1.VSphereMachineProviderSpec
49+
providerStatus *machinev1.VSphereMachineProviderStatus
50+
machineToBePatched runtimeclient.Patch
51+
featureGates featuregate.MutableFeatureGate
5052
}
5153

5254
// newMachineScope creates a new machineScope from the supplied parameters.
@@ -88,16 +90,16 @@ func newMachineScope(params machineScopeParams) (*machineScope, error) {
8890
}
8991

9092
return &machineScope{
91-
Context: params.Context,
92-
client: params.client,
93-
apiReader: params.apiReader,
94-
session: authSession,
95-
machine: params.machine,
96-
providerSpec: providerSpec,
97-
providerStatus: providerStatus,
98-
vSphereConfig: vSphereConfig,
99-
staticIPFeatureGateEnabled: params.StaticIPFeatureGateEnabled,
100-
machineToBePatched: runtimeclient.MergeFrom(params.machine.DeepCopy()),
93+
Context: params.Context,
94+
client: params.client,
95+
apiReader: params.apiReader,
96+
session: authSession,
97+
machine: params.machine,
98+
providerSpec: providerSpec,
99+
providerStatus: providerStatus,
100+
vSphereConfig: vSphereConfig,
101+
featureGates: params.featureGates,
102+
machineToBePatched: runtimeclient.MergeFrom(params.machine.DeepCopy()),
101103
}, nil
102104
}
103105

‎pkg/controller/vsphere/reconciler.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@ import (
2727
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2828
apimachinerytypes "k8s.io/apimachinery/pkg/types"
2929
apimachineryutilerrors "k8s.io/apimachinery/pkg/util/errors"
30+
"k8s.io/component-base/featuregate"
3031
"k8s.io/klog/v2"
3132
"k8s.io/utils/ptr"
3233

34+
apifeatures "github.com/openshift/api/features"
3335
machinev1 "github.com/openshift/api/machine/v1beta1"
3436

3537
machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
@@ -82,7 +84,7 @@ func (r *Reconciler) create() error {
8284
}
8385

8486
if ipam.HasStaticIPConfiguration(r.providerSpec) {
85-
if !r.staticIPFeatureGateEnabled {
87+
if !r.featureGates.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereStaticIPs)) {
8688
return fmt.Errorf("%v: static IP/IPAM configuration is only available with the VSphereStaticIPs feature gate", r.machine.GetName())
8789
}
8890

@@ -389,7 +391,7 @@ func (r *Reconciler) delete() error {
389391
return err
390392
}
391393
klog.Infof("%v: vm does not exist", r.machine.GetName())
392-
if r.staticIPFeatureGateEnabled {
394+
if r.featureGates.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereStaticIPs)) {
393395
// remove any finalizers for IPAddressClaims which may be associated with the machine
394396
err = ipam.RemoveFinalizersForIPAddressClaims(r.Context, r.client, *r.machine)
395397
if err != nil {
@@ -986,7 +988,7 @@ func clone(s *machineScope) (string, error) {
986988
Value: "TRUE",
987989
})
988990

989-
if s.staticIPFeatureGateEnabled {
991+
if s.featureGates.Enabled(featuregate.Feature(apifeatures.FeatureGateVSphereStaticIPs)) {
990992
if ipam.HasStaticIPConfiguration(s.providerSpec) {
991993
networkKargs, err := constructKargsFromNetworkConfig(s)
992994
if err != nil {

‎pkg/controller/vsphere/reconciler_test.go

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,12 @@ import (
4444
"sigs.k8s.io/controller-runtime/pkg/client/fake"
4545

4646
configv1 "github.com/openshift/api/config/v1"
47+
"github.com/openshift/api/features"
4748
machinev1 "github.com/openshift/api/machine/v1beta1"
4849

4950
machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
5051
"github.com/openshift/machine-api-operator/pkg/controller/vsphere/session"
52+
testutils "github.com/openshift/machine-api-operator/pkg/util/testing"
5153
ipamv1beta1 "sigs.k8s.io/cluster-api/exp/ipam/api/v1beta1"
5254

5355
_ "github.com/vmware/govmomi/vapi/simulator"
@@ -178,6 +180,7 @@ func TestClone(t *testing.T) {
178180
}
179181

180182
getMachineScope := func(providerSpec *machinev1.VSphereMachineProviderSpec) *machineScope {
183+
gates, _ := testutils.NewDefaultMutableFeatureGate()
181184
return &machineScope{
182185
Context: context.TODO(),
183186
machine: &machinev1.Machine{
@@ -193,6 +196,7 @@ func TestClone(t *testing.T) {
193196
session: session,
194197
providerStatus: &machinev1.VSphereMachineProviderStatus{},
195198
client: fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(&credentialsSecret, &userDataSecret).Build(),
199+
featureGates: gates,
196200
}
197201
}
198202

@@ -1949,6 +1953,11 @@ func TestDelete(t *testing.T) {
19491953
t.Fatal(err)
19501954
}
19511955

1956+
gates, err := testutils.NewDefaultMutableFeatureGate()
1957+
if err != nil {
1958+
t.Errorf("Unexpected error setting up feature gates: %v", err)
1959+
}
1960+
19521961
client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(
19531962
simParams.secret,
19541963
tc.machine(t, simParams.host),
@@ -1961,6 +1970,7 @@ func TestDelete(t *testing.T) {
19611970
machine: tc.machine(t, simParams.host),
19621971
apiReader: client,
19631972
openshiftConfigNameSpace: openshiftConfigNamespaceForTest,
1973+
featureGates: gates,
19641974
})
19651975
if err != nil {
19661976
t.Fatal(err)
@@ -2610,13 +2620,22 @@ func TestCreate(t *testing.T) {
26102620

26112621
client := builder.Build()
26122622

2623+
gates, err := testutils.NewDefaultMutableFeatureGate()
2624+
if err != nil {
2625+
t.Errorf("Unexpected error setting up feature gates: %v", err)
2626+
}
2627+
2628+
if err := gates.Set(fmt.Sprintf("%v=%v", features.FeatureGateVSphereStaticIPs, tc.staticIPFeatureGateEnabled)); err != nil {
2629+
t.Errorf("Unexpected error setting static IP feature gates: %v", err)
2630+
}
2631+
26132632
machineScope, err := newMachineScope(machineScopeParams{
2614-
client: client,
2615-
Context: context.Background(),
2616-
machine: machine,
2617-
apiReader: client,
2618-
StaticIPFeatureGateEnabled: tc.staticIPFeatureGateEnabled,
2619-
openshiftConfigNameSpace: openshiftConfigNamespaceForTest,
2633+
client: client,
2634+
Context: context.Background(),
2635+
machine: machine,
2636+
apiReader: client,
2637+
openshiftConfigNameSpace: openshiftConfigNamespaceForTest,
2638+
featureGates: gates,
26202639
})
26212640
if err != nil {
26222641
t.Fatal(err)

‎pkg/util/testing/testing.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func NewMachineHealthCheck(name string) *machinev1.MachineHealthCheck {
178178

179179
func NewDefaultMutableFeatureGate() (featuregate.MutableFeatureGate, error) {
180180
defaultMutableGate := feature.DefaultMutableFeatureGate
181-
_, err := features.NewFeatureGateOptions(defaultMutableGate, openshiftfeatures.SelfManaged, openshiftfeatures.FeatureGateMachineAPIMigration)
181+
_, err := features.NewFeatureGateOptions(defaultMutableGate, openshiftfeatures.SelfManaged, openshiftfeatures.FeatureGateMachineAPIMigration, openshiftfeatures.FeatureGateVSphereStaticIPs)
182182
if err != nil {
183183
return nil, fmt.Errorf("failed to set up default feature gate: %w", err)
184184
}

0 commit comments

Comments
 (0)