Skip to content

Introduce protectedCopiedCSVNamespaces flag #2811

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

Merged
merged 1 commit into from
Jul 21, 2022
Merged

Introduce protectedCopiedCSVNamespaces flag #2811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions cmd/olm/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ var (
tlsKeyPath = pflag.String(
"tls-key", "", "Path to use for private key (requires tls-cert)")

protectedCopiedCSVNamespaces = pflag.String("protectedCopiedCSVNamespaces",
"", "A comma-delimited set of namespaces where global Copied CSVs will always appear, even if Copied CSVs are disabled")

tlsCertPath = pflag.String(
"tls-cert", "", "Path to use for certificate key (requires tls-key)")

Expand Down Expand Up @@ -162,6 +165,7 @@ func main() {
olm.WithOperatorClient(opClient),
olm.WithRestConfig(config),
olm.WithConfigClient(versionedConfigClient),
olm.WithProtectedCopiedCSVNamespaces(*protectedCopiedCSVNamespaces),
)
if err != nil {
logger.WithError(err).Fatal("error configuring operator")
Expand Down
55 changes: 35 additions & 20 deletions pkg/controller/operators/olm/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package olm

import (
"strings"
"time"

"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
Expand All @@ -21,18 +22,19 @@ import (
type OperatorOption func(*operatorConfig)

type operatorConfig struct {
resyncPeriod func() time.Duration
operatorNamespace string
watchedNamespaces []string
clock utilclock.Clock
logger *logrus.Logger
operatorClient operatorclient.ClientInterface
externalClient versioned.Interface
strategyResolver install.StrategyResolverInterface
apiReconciler APIIntersectionReconciler
apiLabeler labeler.Labeler
restConfig *rest.Config
configClient configv1client.Interface
protectedCopiedCSVNamespaces map[string]struct{}
resyncPeriod func() time.Duration
operatorNamespace string
watchedNamespaces []string
clock utilclock.Clock
logger *logrus.Logger
operatorClient operatorclient.ClientInterface
externalClient versioned.Interface
strategyResolver install.StrategyResolverInterface
apiReconciler APIIntersectionReconciler
apiLabeler labeler.Labeler
restConfig *rest.Config
configClient configv1client.Interface
}

func (o *operatorConfig) apply(options []OperatorOption) {
Expand Down Expand Up @@ -77,14 +79,15 @@ func (o *operatorConfig) validate() (err error) {

func defaultOperatorConfig() *operatorConfig {
return &operatorConfig{
resyncPeriod: queueinformer.ResyncWithJitter(30*time.Second, 0.2),
operatorNamespace: "default",
watchedNamespaces: []string{metav1.NamespaceAll},
clock: utilclock.RealClock{},
logger: logrus.New(),
strategyResolver: &install.StrategyResolver{},
apiReconciler: APIIntersectionReconcileFunc(ReconcileAPIIntersection),
apiLabeler: labeler.Func(LabelSetsFor),
resyncPeriod: queueinformer.ResyncWithJitter(30*time.Second, 0.2),
operatorNamespace: "default",
watchedNamespaces: []string{metav1.NamespaceAll},
clock: utilclock.RealClock{},
logger: logrus.New(),
strategyResolver: &install.StrategyResolver{},
apiReconciler: APIIntersectionReconcileFunc(ReconcileAPIIntersection),
apiLabeler: labeler.Func(LabelSetsFor),
protectedCopiedCSVNamespaces: map[string]struct{}{},
}
}

Expand Down Expand Up @@ -112,6 +115,18 @@ func WithLogger(logger *logrus.Logger) OperatorOption {
}
}

func WithProtectedCopiedCSVNamespaces(namespaces string) OperatorOption {
return func(config *operatorConfig) {
if namespaces == "" {
return
}

for _, ns := range strings.Split(namespaces, ",") {
config.protectedCopiedCSVNamespaces[ns] = struct{}{}
}
}
}

func WithClock(clock utilclock.Clock) OperatorOption {
return func(config *operatorConfig) {
config.clock = clock
Expand Down
184 changes: 122 additions & 62 deletions pkg/controller/operators/olm/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,32 +63,33 @@ var (
type Operator struct {
queueinformer.Operator

clock utilclock.Clock
logger *logrus.Logger
opClient operatorclient.ClientInterface
client versioned.Interface
lister operatorlister.OperatorLister
copiedCSVLister operatorsv1alpha1listers.ClusterServiceVersionLister
ogQueueSet *queueinformer.ResourceQueueSet
csvQueueSet *queueinformer.ResourceQueueSet
olmConfigQueue workqueue.RateLimitingInterface
csvCopyQueueSet *queueinformer.ResourceQueueSet
copiedCSVGCQueueSet *queueinformer.ResourceQueueSet
objGCQueueSet *queueinformer.ResourceQueueSet
nsQueueSet workqueue.RateLimitingInterface
apiServiceQueue workqueue.RateLimitingInterface
csvIndexers map[string]cache.Indexer
recorder record.EventRecorder
resolver install.StrategyResolverInterface
apiReconciler APIIntersectionReconciler
apiLabeler labeler.Labeler
csvSetGenerator csvutility.SetGenerator
csvReplaceFinder csvutility.ReplaceFinder
csvNotification csvutility.WatchNotification
serviceAccountSyncer *scoped.UserDefinedServiceAccountSyncer
clientAttenuator *scoped.ClientAttenuator
serviceAccountQuerier *scoped.UserDefinedServiceAccountQuerier
clientFactory clients.Factory
clock utilclock.Clock
logger *logrus.Logger
opClient operatorclient.ClientInterface
client versioned.Interface
lister operatorlister.OperatorLister
protectedCopiedCSVNamespaces map[string]struct{}
copiedCSVLister operatorsv1alpha1listers.ClusterServiceVersionLister
ogQueueSet *queueinformer.ResourceQueueSet
csvQueueSet *queueinformer.ResourceQueueSet
olmConfigQueue workqueue.RateLimitingInterface
csvCopyQueueSet *queueinformer.ResourceQueueSet
copiedCSVGCQueueSet *queueinformer.ResourceQueueSet
objGCQueueSet *queueinformer.ResourceQueueSet
nsQueueSet workqueue.RateLimitingInterface
apiServiceQueue workqueue.RateLimitingInterface
csvIndexers map[string]cache.Indexer
recorder record.EventRecorder
resolver install.StrategyResolverInterface
apiReconciler APIIntersectionReconciler
apiLabeler labeler.Labeler
csvSetGenerator csvutility.SetGenerator
csvReplaceFinder csvutility.ReplaceFinder
csvNotification csvutility.WatchNotification
serviceAccountSyncer *scoped.UserDefinedServiceAccountSyncer
clientAttenuator *scoped.ClientAttenuator
serviceAccountQuerier *scoped.UserDefinedServiceAccountQuerier
clientFactory clients.Factory
}

func NewOperator(ctx context.Context, options ...OperatorOption) (*Operator, error) {
Expand Down Expand Up @@ -121,30 +122,31 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
}

op := &Operator{
Operator: queueOperator,
clock: config.clock,
logger: config.logger,
opClient: config.operatorClient,
client: config.externalClient,
ogQueueSet: queueinformer.NewEmptyResourceQueueSet(),
csvQueueSet: queueinformer.NewEmptyResourceQueueSet(),
olmConfigQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "olmConfig"),
csvCopyQueueSet: queueinformer.NewEmptyResourceQueueSet(),
copiedCSVGCQueueSet: queueinformer.NewEmptyResourceQueueSet(),
objGCQueueSet: queueinformer.NewEmptyResourceQueueSet(),
apiServiceQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "apiservice"),
resolver: config.strategyResolver,
apiReconciler: config.apiReconciler,
lister: lister,
recorder: eventRecorder,
apiLabeler: config.apiLabeler,
csvIndexers: map[string]cache.Indexer{},
csvSetGenerator: csvutility.NewSetGenerator(config.logger, lister),
csvReplaceFinder: csvutility.NewReplaceFinder(config.logger, config.externalClient),
serviceAccountSyncer: scoped.NewUserDefinedServiceAccountSyncer(config.logger, scheme, config.operatorClient, config.externalClient),
clientAttenuator: scoped.NewClientAttenuator(config.logger, config.restConfig, config.operatorClient),
serviceAccountQuerier: scoped.NewUserDefinedServiceAccountQuerier(config.logger, config.externalClient),
clientFactory: clients.NewFactory(config.restConfig),
Operator: queueOperator,
clock: config.clock,
logger: config.logger,
opClient: config.operatorClient,
client: config.externalClient,
ogQueueSet: queueinformer.NewEmptyResourceQueueSet(),
csvQueueSet: queueinformer.NewEmptyResourceQueueSet(),
olmConfigQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "olmConfig"),
csvCopyQueueSet: queueinformer.NewEmptyResourceQueueSet(),
copiedCSVGCQueueSet: queueinformer.NewEmptyResourceQueueSet(),
objGCQueueSet: queueinformer.NewEmptyResourceQueueSet(),
apiServiceQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "apiservice"),
resolver: config.strategyResolver,
apiReconciler: config.apiReconciler,
lister: lister,
recorder: eventRecorder,
apiLabeler: config.apiLabeler,
csvIndexers: map[string]cache.Indexer{},
csvSetGenerator: csvutility.NewSetGenerator(config.logger, lister),
csvReplaceFinder: csvutility.NewReplaceFinder(config.logger, config.externalClient),
serviceAccountSyncer: scoped.NewUserDefinedServiceAccountSyncer(config.logger, scheme, config.operatorClient, config.externalClient),
clientAttenuator: scoped.NewClientAttenuator(config.logger, config.restConfig, config.operatorClient),
serviceAccountQuerier: scoped.NewUserDefinedServiceAccountQuerier(config.logger, config.externalClient),
clientFactory: clients.NewFactory(config.restConfig),
protectedCopiedCSVNamespaces: config.protectedCopiedCSVNamespaces,
}

// Set up syncing for namespace-scoped resources
Expand Down Expand Up @@ -1299,20 +1301,31 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) {
return err
}

// Filter to unique copies
uniqueCopiedCSVs := map[string]struct{}{}
// Create a map that points from CSV name to a map of namespaces it is copied to
// for quick lookups.
copiedCSVNamespaces := map[string]map[string]struct{}{}
for _, copiedCSV := range copiedCSVs {
uniqueCopiedCSVs[copiedCSV.GetName()] = struct{}{}
if _, ok := copiedCSVNamespaces[copiedCSV.GetName()]; !ok {
copiedCSVNamespaces[copiedCSV.GetName()] = map[string]struct{}{}
}
copiedCSVNamespaces[copiedCSV.GetName()][copiedCSV.GetNamespace()] = struct{}{}
}

csvs, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(og.GetNamespace()).List(labels.NewSelector().Add(*nonCopiedCSVRequirement))
if err != nil {
return err
}

namespaces, err := a.lister.CoreV1().NamespaceLister().List(labels.Everything())
if err != nil {
return err
}

copiedCSVEvaluatorFunc := getCopiedCSVEvaluatorFunc(olmConfig.CopiedCSVsAreEnabled(), namespaces, a.protectedCopiedCSVNamespaces)

for _, csv := range csvs {
// If the correct number of copied CSVs were found, continue
if _, ok := uniqueCopiedCSVs[csv.GetName()]; ok == olmConfig.CopiedCSVsAreEnabled() {
// Ignore NS where actual CSV is installed
if copiedCSVEvaluatorFunc(copiedCSVNamespaces[csv.GetName()]) {
continue
}

Expand All @@ -1324,7 +1337,7 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) {
}

// Update the olmConfig status if it has changed.
condition := getCopiedCSVsCondition(!olmConfig.CopiedCSVsAreEnabled(), csvIsRequeued)
condition := getCopiedCSVsCondition(olmConfig.CopiedCSVsAreEnabled(), csvIsRequeued)
if !isStatusConditionPresentAndAreTypeReasonMessageStatusEqual(olmConfig.Status.Conditions, condition) {
meta.SetStatusCondition(&olmConfig.Status.Conditions, condition)
if _, err := a.client.OperatorsV1().OLMConfigs().UpdateStatus(context.TODO(), olmConfig, metav1.UpdateOptions{}); err != nil {
Expand All @@ -1335,6 +1348,37 @@ func (a *Operator) syncOLMConfig(obj interface{}) (syncError error) {
return nil
}

// getCopiedCSVEvaluatorFunc returns a function that evaluates if the a set of Copied CSVs exist in the expected namespaces.
func getCopiedCSVEvaluatorFunc(copiedCSVsEnabled bool, namespaces []*corev1.Namespace, protectedCopiedCSVNamespaces map[string]struct{}) func(map[string]struct{}) bool {
if copiedCSVsEnabled {
// Exclude the namespace hosting the original CSV
expectedCopiedCSVCount := -1
for _, ns := range namespaces {
if ns.Status.Phase == corev1.NamespaceActive {
expectedCopiedCSVCount++
}
}
return func(m map[string]struct{}) bool {
return expectedCopiedCSVCount == len(m)
}
}

// Check that Copied CSVs exist in protected namespaces.
return func(m map[string]struct{}) bool {
if len(protectedCopiedCSVNamespaces) != len(m) {
return false
}

for protectedNS := range protectedCopiedCSVNamespaces {
if _, ok := m[protectedNS]; !ok {
return false
}
}

return true
}
}

func isStatusConditionPresentAndAreTypeReasonMessageStatusEqual(conditions []metav1.Condition, condition metav1.Condition) bool {
foundCondition := meta.FindStatusCondition(conditions, condition.Type)
if foundCondition == nil {
Expand All @@ -1346,13 +1390,13 @@ func isStatusConditionPresentAndAreTypeReasonMessageStatusEqual(conditions []met
foundCondition.Status == condition.Status
}

func getCopiedCSVsCondition(isDisabled, csvIsRequeued bool) metav1.Condition {
func getCopiedCSVsCondition(enabled, csvIsRequeued bool) metav1.Condition {
condition := metav1.Condition{
Type: operatorsv1.DisabledCopiedCSVsConditionType,
LastTransitionTime: metav1.Now(),
Status: metav1.ConditionFalse,
}
if !isDisabled {
if enabled {
condition.Reason = "CopiedCSVsEnabled"
condition.Message = "Copied CSVs are enabled and present across the cluster"
if csvIsRequeued {
Expand All @@ -1361,15 +1405,14 @@ func getCopiedCSVsCondition(isDisabled, csvIsRequeued bool) metav1.Condition {
return condition
}

condition.Reason = "CopiedCSVsDisabled"
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to live as a constant? Where are the reasons typically defined, o-f/api?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's probably a good idea, any objections to doing this in a followup PR if I create an issue to track it?

Copy link
Member

Choose a reason for hiding this comment

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

Just saw this: that's fine with me, but I'd like to see us tackle this sooner than later.

if csvIsRequeued {
condition.Reason = "CopiedCSVsFound"
condition.Message = "Copied CSVs are disabled and at least one copied CSV was found for an operator installed in AllNamespace mode"
condition.Message = "Copied CSVs are disabled and at least one unexpected copied CSV was found for an operator installed in AllNamespace mode"
return condition
}

condition.Status = metav1.ConditionTrue
condition.Reason = "NoCopiedCSVsFound"
condition.Message = "Copied CSVs are disabled and none were found for operators installed in AllNamespace mode"
condition.Message = "Copied CSVs are disabled and no unexpected copied CSVs were found for operators installed in AllNamespace mode"

return condition
}
Expand Down Expand Up @@ -1444,7 +1487,24 @@ func (a *Operator) syncCopyCSV(obj interface{}) (syncError error) {
return err
}

// Ensure that the Copied CSVs exist in the protected namespaces.
protectedNamespaces := []string{}
for ns := range a.protectedCopiedCSVNamespaces {
if ns == clusterServiceVersion.GetNamespace() {
continue
}
protectedNamespaces = append(protectedNamespaces, ns)
}

if err := a.ensureCSVsInNamespaces(clusterServiceVersion, operatorGroup, NewNamespaceSet(protectedNamespaces)); err != nil {
return err
}

// Delete Copied CSVs in namespaces that are not protected.
for _, copiedCSV := range copiedCSVs {
if _, ok := a.protectedCopiedCSVNamespaces[copiedCSV.Namespace]; ok {
continue
}
err := a.client.OperatorsV1alpha1().ClusterServiceVersions(copiedCSV.Namespace).Delete(context.TODO(), copiedCSV.Name, metav1.DeleteOptions{})
if err != nil && !apierrors.IsNotFound(err) {
return err
Expand Down
Loading