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

precondition checker for reducing encryption QPS #1059

Merged
merged 5 commits into from May 7, 2021
Merged

precondition checker for reducing encryption QPS #1059

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ed6496b
provides a precondition checker determines if encryption controllers …
p0lyn0mial Apr 27, 2021
9daa8a3
add unit tests for the preconditions checker
p0lyn0mial May 6, 2021
6969150
wire the preconditions checker to the encryption controllers
p0lyn0mial May 6, 2021
cb236f2
apiservercontrollerset: propagate errors that occurred during object/…
p0lyn0mial May 6, 2021
a503000
with encryptionWasEnabled
p0lyn0mial May 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/operator/apiserver/controllerset/apiservercontrollerset.go
View file
Open in desktop
Expand Up @@ -43,6 +43,8 @@ type preparedAPIServerControllerSet struct {

type controllerWrapper struct {
emptyAllowed bool
// creationError allows for reporting errors that occurred during object creation
creationError error
controller
}

Expand All @@ -54,6 +56,9 @@ func (cw *controllerWrapper) prepare() (controller, error) {
if !cw.emptyAllowed && cw.controller == nil {
return nil, fmt.Errorf("missing controller")
}
if cw.creationError != nil {
return nil, cw.creationError
}

return cw.controller, nil
}
Expand Down Expand Up @@ -401,7 +406,7 @@ func (e *encryptionControllerBuilder) build() controllerWrapper {
if e.emptyAllowed {
return e.controllerWrapper
}
e.controllerWrapper.controller = encryption.NewControllers(
e.controllerWrapper.controller, e.controllerWrapper.creationError = encryption.NewControllers(
e.component,
e.unsupportedConfigPrefix,
e.provider,
Expand Down
14 changes: 12 additions & 2 deletions pkg/operator/encryption/controllers.go
View file
Open in desktop
Expand Up @@ -35,20 +35,26 @@ func NewControllers(
kubeInformersForNamespaces operatorv1helpers.KubeInformersForNamespaces,
secretsClient corev1.SecretsGetter,
eventRecorder events.Recorder,
) *Controllers {
) (*Controllers, error) {
// avoid using the CachedSecretGetter as we need strong guarantees that our encryptionSecretSelector works
// otherwise we could see secrets from a different component (which will break our keyID invariants)
// this is fine in terms of performance since these controllers will be idle most of the time
// TODO: update the eventHandlers used by the controllers to ignore components that do not match their own
encryptionSecretSelector := metav1.ListOptions{LabelSelector: secrets.EncryptionKeySecretsLabel + "=" + component}

encryptionEnabledChecker, err := newEncryptionEnabledPrecondition(apiServerInformer, kubeInformersForNamespaces, encryptionSecretSelector.LabelSelector)
if err != nil {
return nil, err
}

return &Controllers{
controllers: []runner{
controllers.NewKeyController(
component,
unsupportedConfigPrefix,
provider,
deployer,
encryptionEnabledChecker.PreconditionFulfilled,
operatorClient,
apiServerClient,
apiServerInformer,
Expand All @@ -61,6 +67,7 @@ func NewControllers(
component,
provider,
deployer,
encryptionEnabledChecker.PreconditionFulfilled,
operatorClient,
kubeInformersForNamespaces,
secretsClient,
Expand All @@ -70,6 +77,7 @@ func NewControllers(
controllers.NewPruneController(
provider,
deployer,
encryptionEnabledChecker.PreconditionFulfilled,
operatorClient,
kubeInformersForNamespaces,
secretsClient,
Expand All @@ -80,6 +88,7 @@ func NewControllers(
component,
provider,
deployer,
encryptionEnabledChecker.PreconditionFulfilled,
migrator,
operatorClient,
kubeInformersForNamespaces,
Expand All @@ -90,14 +99,15 @@ func NewControllers(
controllers.NewConditionController(
provider,
deployer,
encryptionEnabledChecker.PreconditionFulfilled,
operatorClient,
kubeInformersForNamespaces,
secretsClient,
encryptionSecretSelector,
eventRecorder,
),
},
}
}, nil
}

type Controllers struct {
Expand Down
11 changes: 7 additions & 4 deletions pkg/operator/encryption/controllers/condition_controller.go
View file
Open in desktop
Expand Up @@ -27,14 +27,16 @@ type conditionController struct {

encryptionSecretSelector metav1.ListOptions

deployer statemachine.Deployer
provider Provider
secretClient corev1client.SecretsGetter
deployer statemachine.Deployer
provider Provider
preconditionsFulfilledFn preconditionsFulfilled
secretClient corev1client.SecretsGetter
}

func NewConditionController(
provider Provider,
deployer statemachine.Deployer,
preconditionsFulfilledFn preconditionsFulfilled,
operatorClient operatorv1helpers.OperatorClient,
kubeInformersForNamespaces operatorv1helpers.KubeInformersForNamespaces,
secretClient corev1client.SecretsGetter,
Expand All @@ -47,6 +49,7 @@ func NewConditionController(
encryptionSecretSelector: encryptionSecretSelector,
deployer: deployer,
provider: provider,
preconditionsFulfilledFn: preconditionsFulfilledFn,
secretClient: secretClient,
}

Expand All @@ -58,7 +61,7 @@ func NewConditionController(
}

func (c *conditionController) sync(ctx context.Context, syncContext factory.SyncContext) error {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.preconditionsFulfilledFn, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
return err // we will get re-kicked when the operator status updates
}

Expand Down
12 changes: 9 additions & 3 deletions pkg/operator/encryption/controllers/controller.go
View file
Open in desktop
Expand Up @@ -7,6 +7,9 @@ import (
operatorv1helpers "github.com/openshift/library-go/pkg/operator/v1helpers"
)

// preconditionsFulfilled a function that indicates whether all prerequisites are met and we can Sync.
type preconditionsFulfilled func() (bool, error)

// Provider abstracts external dependencies and preconditions that need to be dynamic during a downgrade/upgrade
type Provider interface {
// EncryptedGRs returns resources that need to be encrypted
Expand All @@ -16,8 +19,7 @@ type Provider interface {
ShouldRunEncryptionControllers() (bool, error)
}

func shouldRunEncryptionController(operatorClient operatorv1helpers.OperatorClient, shouldRunFn func() (bool, error)) (bool, error) {

func shouldRunEncryptionController(operatorClient operatorv1helpers.OperatorClient, preconditionsFulfilledFn preconditionsFulfilled, shouldRunFn func() (bool, error)) (bool, error) {
if shouldRun, err := shouldRunFn(); !shouldRun || err != nil {
return false, err
}
Expand All @@ -27,5 +29,9 @@ func shouldRunEncryptionController(operatorClient operatorv1helpers.OperatorClie
return false, err
}

return management.IsOperatorManaged(operatorSpec.ManagementState), nil
if !management.IsOperatorManaged(operatorSpec.ManagementState) {
return false, nil
}

return preconditionsFulfilledFn()
}
2 changes: 2 additions & 0 deletions pkg/operator/encryption/controllers/helpers_test.go
View file
Open in desktop
Expand Up @@ -21,6 +21,8 @@ func createEncryptionCfgSecret(t *testing.T, targetNs string, revision string, e
return s
}

var alwaysFulfilledPreconditions = func() (bool, error) { return true, nil }

type testProvider struct {
encryptedGRs []schema.GroupResource
}
Expand Down
11 changes: 7 additions & 4 deletions pkg/operator/encryption/controllers/key_controller.go
View file
Open in desktop
Expand Up @@ -65,9 +65,10 @@ type keyController struct {
name string
encryptionSecretSelector metav1.ListOptions

deployer statemachine.Deployer
secretClient corev1client.SecretsGetter
provider Provider
deployer statemachine.Deployer
secretClient corev1client.SecretsGetter
provider Provider
preconditionsFulfilledFn preconditionsFulfilled

unsupportedConfigPrefix []string
}
Expand All @@ -77,6 +78,7 @@ func NewKeyController(
unsupportedConfigPrefix []string,
provider Provider,
deployer statemachine.Deployer,
preconditionsFulfilledFn preconditionsFulfilled,
operatorClient operatorv1helpers.OperatorClient,
apiServerClient configv1client.APIServerInterface,
apiServerInformer configv1informers.APIServerInformer,
Expand All @@ -96,6 +98,7 @@ func NewKeyController(
encryptionSecretSelector: encryptionSecretSelector,
deployer: deployer,
provider: provider,
preconditionsFulfilledFn: preconditionsFulfilledFn,
secretClient: secretClient,
}

Expand All @@ -111,7 +114,7 @@ func NewKeyController(
}

func (c *keyController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.preconditionsFulfilledFn, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
return err // we will get re-kicked when the operator status updates
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/encryption/controllers/key_controller_test.go
View file
Open in desktop
Expand Up @@ -343,7 +343,7 @@ func TestKeyController(t *testing.T) {
}
provider := newTestProvider(scenario.targetGRs)

target := NewKeyController(scenario.targetNamespace, nil, provider, deployer, fakeOperatorClient, fakeApiServerClient, fakeApiServerInformer, kubeInformers, fakeSecretClient, scenario.encryptionSecretSelector, eventRecorder)
target := NewKeyController(scenario.targetNamespace, nil, provider, deployer, alwaysFulfilledPreconditions, fakeOperatorClient, fakeApiServerClient, fakeApiServerInformer, kubeInformers, fakeSecretClient, scenario.encryptionSecretSelector, eventRecorder)

// act
err = target.Sync(context.TODO(), factory.NewSyncContext("test", eventRecorder))
Expand Down
11 changes: 7 additions & 4 deletions pkg/operator/encryption/controllers/migration_controller.go
View file
Open in desktop
Expand Up @@ -62,15 +62,17 @@ type migrationController struct {
preRunCachesSynced []cache.InformerSynced
encryptionSecretSelector metav1.ListOptions

deployer statemachine.Deployer
migrator migrators.Migrator
provider Provider
deployer statemachine.Deployer
migrator migrators.Migrator
provider Provider
preconditionsFulfilledFn preconditionsFulfilled
}

func NewMigrationController(
component string,
provider Provider,
deployer statemachine.Deployer,
preconditionsFulfilledFn preconditionsFulfilled,
migrator migrators.Migrator,
operatorClient operatorv1helpers.OperatorClient,
kubeInformersForNamespaces operatorv1helpers.KubeInformersForNamespaces,
Expand All @@ -88,6 +90,7 @@ func NewMigrationController(
deployer: deployer,
migrator: migrator,
provider: provider,
preconditionsFulfilledFn: preconditionsFulfilledFn,
}

return factory.New().ResyncEvery(time.Minute).WithSync(c.sync).WithInformers(
Expand All @@ -99,7 +102,7 @@ func NewMigrationController(
}

func (c *migrationController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.preconditionsFulfilledFn, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
return err // we will get re-kicked when the operator status updates
}

Expand Down
1 change: 1 addition & 0 deletions pkg/operator/encryption/controllers/migration_controller_test.go
View file
Open in desktop
Expand Up @@ -647,6 +647,7 @@ func TestMigrationController(t *testing.T) {
"kms",
provider,
deployer,
alwaysFulfilledPreconditions,
migrator,
fakeOperatorClient,
kubeInformers,
Expand Down
13 changes: 8 additions & 5 deletions pkg/operator/encryption/controllers/prune_controller.go
View file
Open in desktop
Expand Up @@ -39,15 +39,17 @@ type pruneController struct {

encryptionSecretSelector metav1.ListOptions

deployer statemachine.Deployer
provider Provider
secretClient corev1client.SecretsGetter
name string
deployer statemachine.Deployer
provider Provider
preconditionsFulfilledFn preconditionsFulfilled
secretClient corev1client.SecretsGetter
name string
}

func NewPruneController(
provider Provider,
deployer statemachine.Deployer,
preconditionsFulfilledFn preconditionsFulfilled,
operatorClient operatorv1helpers.OperatorClient,
kubeInformersForNamespaces operatorv1helpers.KubeInformersForNamespaces,
secretClient corev1client.SecretsGetter,
Expand All @@ -60,6 +62,7 @@ func NewPruneController(
encryptionSecretSelector: encryptionSecretSelector,
deployer: deployer,
provider: provider,
preconditionsFulfilledFn: preconditionsFulfilledFn,
secretClient: secretClient,
}

Expand All @@ -71,7 +74,7 @@ func NewPruneController(
}

func (c *pruneController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.preconditionsFulfilledFn, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
return err // we will get re-kicked when the operator status updates
}

Expand Down
1 change: 1 addition & 0 deletions pkg/operator/encryption/controllers/prune_controller_test.go
View file
Open in desktop
Expand Up @@ -182,6 +182,7 @@ func TestPruneController(t *testing.T) {
target := NewPruneController(
provider,
deployer,
alwaysFulfilledPreconditions,
fakeOperatorClient,
kubeInformers,
fakeSecretClient,
Expand Down
13 changes: 8 additions & 5 deletions pkg/operator/encryption/controllers/state_controller.go
View file
Open in desktop
Expand Up @@ -40,16 +40,18 @@ type stateController struct {
name string
encryptionSecretSelector metav1.ListOptions

operatorClient operatorv1helpers.OperatorClient
secretClient corev1client.SecretsGetter
deployer statemachine.Deployer
provider Provider
operatorClient operatorv1helpers.OperatorClient
secretClient corev1client.SecretsGetter
deployer statemachine.Deployer
provider Provider
preconditionsFulfilledFn preconditionsFulfilled
}

func NewStateController(
component string,
provider Provider,
deployer statemachine.Deployer,
preconditionsFulfilledFn preconditionsFulfilled,
operatorClient operatorv1helpers.OperatorClient,
kubeInformersForNamespaces operatorv1helpers.KubeInformersForNamespaces,
secretClient corev1client.SecretsGetter,
Expand All @@ -66,6 +68,7 @@ func NewStateController(
secretClient: secretClient,
deployer: deployer,
provider: provider,
preconditionsFulfilledFn: preconditionsFulfilledFn,
}

return factory.New().ResyncEvery(time.Minute).WithSync(c.sync).WithInformers(
Expand All @@ -77,7 +80,7 @@ func NewStateController(
}

func (c *stateController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
if ready, err := shouldRunEncryptionController(c.operatorClient, c.preconditionsFulfilledFn, c.provider.ShouldRunEncryptionControllers); err != nil || !ready {
return err // we will get re-kicked when the operator status updates
}

Expand Down
1 change: 1 addition & 0 deletions pkg/operator/encryption/controllers/state_controller_test.go
View file
Open in desktop
Expand Up @@ -753,6 +753,7 @@ func TestStateController(t *testing.T) {
scenario.targetNamespace,
provider,
deployer,
alwaysFulfilledPreconditions,
fakeOperatorClient,
kubeInformers,
fakeSecretClient,
Expand Down