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.

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

API-1835: separate apply-configuration-live from apply-configuration #718

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions pkg/cmd/mom/apply_configuration_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ func NewApplyConfigurationCommand(streams genericiooptions.IOStreams) *cobra.Com
}

func RunApplyConfiguration(ctx context.Context, input libraryapplyconfiguration.ApplyConfigurationInput) (libraryapplyconfiguration.AllDesiredMutationsGetter, error) {
// TODO initialize dynamic clients, informers, operator clients, and kubeclients from the input to demonstrate.

authenticationOperatorInput, err := operator.CreateOperatorInputFromMOM(ctx, input)
if err != nil {
return nil, fmt.Errorf("unable to configure operator input: %w", err)
Expand Down
33 changes: 33 additions & 0 deletions pkg/cmd/mom/apply_configuration_live_command.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package mom

import (
"context"
"fmt"

"github.com/openshift/cluster-authentication-operator/pkg/operator"

"github.com/openshift/multi-operator-manager/pkg/library/libraryapplyconfiguration"
"github.com/spf13/cobra"
"k8s.io/cli-runtime/pkg/genericiooptions"
)

func NewApplyConfigurationLiveCommand(streams genericiooptions.IOStreams) *cobra.Command {
return libraryapplyconfiguration.NewApplyConfigurationCommand(RunApplyConfiguration, streams)
}

func RunApplyConfigurationLive(ctx context.Context, input libraryapplyconfiguration.ApplyConfigurationInput) (libraryapplyconfiguration.AllDesiredMutationsGetter, error) {
authenticationOperatorInput, err := operator.CreateOperatorInputFromMOM(ctx, input)
if err != nil {
return nil, fmt.Errorf("unable to configure operator input: %w", err)
}
operatorStarter, err := operator.CreateOperatorStarterLive(ctx, authenticationOperatorInput)
if err != nil {
return nil, fmt.Errorf("unable to configure operators: %w", err)
}
var operatorRunError error
if err := operatorStarter.RunOnce(ctx); err != nil {
operatorRunError = fmt.Errorf("unable to run operators: %w", err)
}

return libraryapplyconfiguration.NewApplyConfigurationFromClient(input.MutationTrackingClient.GetMutations()), operatorRunError
}
17 changes: 17 additions & 0 deletions pkg/operator/replacement_starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,3 +306,20 @@ func CreateOperatorStarter(ctx context.Context, authOperatorInput *authenticatio

return ret, nil
}

func CreateOperatorStarterLive(ctx context.Context, authOperatorInput *authenticationOperatorInput) (libraryapplyconfiguration.OperatorStarter, error) {
ret := &libraryapplyconfiguration.SimpleOperatorStarter{
Informers: append([]libraryapplyconfiguration.SimplifiedInformerFactory{}, authOperatorInput.informerFactories...),
}

informerFactories := newInformerFactories(authOperatorInput)
ret.Informers = append(ret.Informers, informerFactories.simplifiedInformerFactories()...)

oauthRunOnceFns, err := prepareOauthOperatorLive(ctx, authOperatorInput, informerFactories)
if err != nil {
return nil, fmt.Errorf("unable to prepare oauth server: %w", err)
}
ret.ControllerRunOnceFns = append(ret.ControllerRunOnceFns, oauthRunOnceFns...)

return ret, nil
}
59 changes: 55 additions & 4 deletions pkg/operator/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,11 @@ func prepareOauthOperator(
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, routerCertsController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, serviceCAController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, staticResourceController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, wellKnownReadyController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, authRouteCheckController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, authServiceCheckController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, authServiceEndpointCheckController.Sync),
// moved to live
//libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, wellKnownReadyController.Sync),
//libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, authRouteCheckController.Sync),
//libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, authServiceCheckController.Sync),
//libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, authServiceEndpointCheckController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, workersAvailableController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, proxyConfigController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, customRouteController.Sync),
Expand Down Expand Up @@ -377,6 +378,56 @@ func prepareOauthOperator(
return runOnceFns, runFns, nil
}

func prepareOauthOperatorLive(
ctx context.Context,
authOperatorInput *authenticationOperatorInput,
informerFactories authenticationOperatorInformerFactories,
) ([]libraryapplyconfiguration.RunOnceFunc, error) {

systemCABundle, err := loadSystemCACertBundle()
if err != nil {
return nil, err
}

wellKnownReadyController := readiness.NewWellKnownReadyController(
"openshift-authentication",
informerFactories.kubeInformersForNamespaces,
informerFactories.operatorConfigInformer,
informerFactories.namespacedOpenshiftAuthenticationRoutes.Route().V1().Routes(),
authOperatorInput.authenticationOperatorClient,
authOperatorInput.eventRecorder,
)

authRouteCheckController := oauthendpoints.NewOAuthRouteCheckController(
authOperatorInput.authenticationOperatorClient,
informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"),
informerFactories.kubeInformersForNamespaces.InformersFor("openshift-config-managed"),
informerFactories.namespacedOpenshiftAuthenticationRoutes.Route().V1().Routes(),
informerFactories.operatorConfigInformer.Config().V1().Ingresses(),
systemCABundle,
authOperatorInput.eventRecorder,
)
authServiceCheckController := oauthendpoints.NewOAuthServiceCheckController(
authOperatorInput.authenticationOperatorClient,
informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"),
authOperatorInput.eventRecorder,
)
authServiceEndpointCheckController := oauthendpoints.NewOAuthServiceEndpointsCheckController(
authOperatorInput.authenticationOperatorClient,
informerFactories.kubeInformersForNamespaces.InformersFor("openshift-authentication"),
authOperatorInput.eventRecorder,
)

runOnceFns := []libraryapplyconfiguration.RunOnceFunc{
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, wellKnownReadyController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, authRouteCheckController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, authServiceCheckController.Sync),
libraryapplyconfiguration.AdaptSyncFn(authOperatorInput.eventRecorder, authServiceEndpointCheckController.Sync),
}

return runOnceFns, nil
}

func prepareOauthAPIServerOperator(
ctx context.Context,
authOperatorInput *authenticationOperatorInput,
Expand Down
5 changes: 5 additions & 0 deletions pkg/operator/workload/sync_openshift_oauth_apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,11 @@ func (c *OAuthAPIServerWorkload) syncDeployment(ctx context.Context, operatorSpe
}
required.Spec.Replicas = masterNodeCount

// TODO MOM this call here is the one that fails. Using the return value of ApplyDeployment to make decisions doesn't work in a MOM world.
// TODO MOM workload.NewController needs to (somehow) use the data read *after* this write, without using data *in* this write.
// TODO MOM this may be possible by storing a hash of the desired content. If the hash of the desired content matches
// TODO MOM then there is no new deployment required. This would not prevent rapid writes to fields we aren't controlling.
// TODO MOM perhaps we actually need to find a way to request MOM inject a generation after a write? It is an operator standard.
Comment on lines +235 to +239
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is sticky. The hash probably won't work. Making MOM aware of writing workload generation may be the only way to achieve parity.

@bertinatto

deployment, _, err := resourceapply.ApplyDeployment(ctx, c.kubeClient.AppsV1(), eventRecorder, required, resourcemerge.ExpectedDeploymentGeneration(required, operatorStatus.Generations))
return deployment, err
}
Expand Down