Skip to content

Commit

Permalink
clean up AzurePathFix controller, removing unecessary informers
Browse files Browse the repository at this point in the history
* drops use of the azurepathfixjob resource generator, as we only need
  to get and delete the job and we mostly benefit from the generator
  pattern when creating objects.

* remove uneccessary informers. those were needed when creating the job
  object, but are no longer relevant for only getting and deleting it.
  • Loading branch information
flavianmissi authored and openshift-cherrypick-robot committed Nov 26, 2024
1 parent 6918d4f commit d0356a0
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 69 deletions.
3 changes: 3 additions & 0 deletions pkg/defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ const (

// InfrastructureResourceName is the name of the infrastructure config resource
InfrastructureResourceName = "cluster"

// AzurePathFixJobName is the job name for the azure-path-fix job
AzurePathFixJobName = "azure-path-fix"
)

var (
Expand Down
72 changes: 10 additions & 62 deletions pkg/operator/azurepathfixcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,8 @@ import (
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
batchv1informers "k8s.io/client-go/informers/batch/v1"
corev1informers "k8s.io/client-go/informers/core/v1"
batchv1client "k8s.io/client-go/kubernetes/typed/batch/v1"
batchv1listers "k8s.io/client-go/listers/batch/v1"
corev1listers "k8s.io/client-go/listers/core/v1"
restclient "k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue"
"k8s.io/klog/v2"
Expand All @@ -26,11 +23,9 @@ import (
configlisters "github.com/openshift/client-go/config/listers/config/v1"
imageregistryv1informers "github.com/openshift/client-go/imageregistry/informers/externalversions/imageregistry/v1"
imageregistryv1listers "github.com/openshift/client-go/imageregistry/listers/imageregistry/v1"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
"github.com/openshift/library-go/pkg/operator/v1helpers"

"github.com/openshift/cluster-image-registry-operator/pkg/defaults"
"github.com/openshift/cluster-image-registry-operator/pkg/resource"
"github.com/openshift/cluster-image-registry-operator/pkg/storage/util"
)

Expand All @@ -39,45 +34,26 @@ type AzurePathFixController struct {
operatorClient v1helpers.OperatorClient
jobLister batchv1listers.JobNamespaceLister
imageRegistryConfigLister imageregistryv1listers.ConfigLister
secretLister corev1listers.SecretNamespaceLister
podLister corev1listers.PodNamespaceLister
infrastructureLister configlisters.InfrastructureLister
proxyLister configlisters.ProxyLister
openshiftConfigLister corev1listers.ConfigMapNamespaceLister
kubeconfig *restclient.Config

cachesToSync []cache.InformerSynced
queue workqueue.RateLimitingInterface

featureGateAccessor featuregates.FeatureGateAccess
}

func NewAzurePathFixController(
kubeconfig *restclient.Config,
batchClient batchv1client.BatchV1Interface,
operatorClient v1helpers.OperatorClient,
jobInformer batchv1informers.JobInformer,
imageRegistryConfigInformer imageregistryv1informers.ConfigInformer,
infrastructureInformer configv1informers.InfrastructureInformer,
secretInformer corev1informers.SecretInformer,
proxyInformer configv1informers.ProxyInformer,
openshiftConfigInformer corev1informers.ConfigMapInformer,
podInformer corev1informers.PodInformer,
featureGateAccessor featuregates.FeatureGateAccess,
) (*AzurePathFixController, error) {
c := &AzurePathFixController{
batchClient: batchClient,
operatorClient: operatorClient,
jobLister: jobInformer.Lister().Jobs(defaults.ImageRegistryOperatorNamespace),
imageRegistryConfigLister: imageRegistryConfigInformer.Lister(),
infrastructureLister: infrastructureInformer.Lister(),
secretLister: secretInformer.Lister().Secrets(defaults.ImageRegistryOperatorNamespace),
podLister: podInformer.Lister().Pods(defaults.ImageRegistryOperatorNamespace),
proxyLister: proxyInformer.Lister(),
openshiftConfigLister: openshiftConfigInformer.Lister().ConfigMaps(defaults.OpenShiftConfigNamespace),
kubeconfig: kubeconfig,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "AzurePathFixController"),
featureGateAccessor: featureGateAccessor,
}

if _, err := jobInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {
Expand All @@ -90,31 +66,10 @@ func NewAzurePathFixController(
}
c.cachesToSync = append(c.cachesToSync, imageRegistryConfigInformer.Informer().HasSynced)

if _, err := infrastructureInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {
return nil, err
}
// we need the infra only to check for PlatformStatus.Type, which isn't
// expected to change, so we don't need to add an event handler.
c.cachesToSync = append(c.cachesToSync, infrastructureInformer.Informer().HasSynced)

if _, err := secretInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {
return nil, err
}
c.cachesToSync = append(c.cachesToSync, secretInformer.Informer().HasSynced)

if _, err := podInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {
return nil, err
}
c.cachesToSync = append(c.cachesToSync, podInformer.Informer().HasSynced)

if _, err := proxyInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {
return nil, err
}
c.cachesToSync = append(c.cachesToSync, proxyInformer.Informer().HasSynced)

if _, err := openshiftConfigInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {
return nil, err
}
c.cachesToSync = append(c.cachesToSync, openshiftConfigInformer.Informer().HasSynced)

// bootstrap the job if it doesn't exist
c.queue.Add("instance")

Expand Down Expand Up @@ -163,6 +118,8 @@ func (c *AzurePathFixController) processNextWorkItem() bool {
}

func (c *AzurePathFixController) sync() error {
ctx := context.TODO()

// this controller was made to run specifically on Azure,
// so if we detect a different cloud, skip it.
infra, err := util.GetInfrastructure(c.infrastructureLister)
Expand All @@ -185,17 +142,6 @@ func (c *AzurePathFixController) sync() error {
return nil
}

gen := resource.NewGeneratorAzurePathFixJob(
c.jobLister,
c.batchClient,
c.secretLister,
c.infrastructureLister,
c.proxyLister,
c.openshiftConfigLister,
imageRegistryConfig,
c.kubeconfig,
)

// this controller was created to aid users migrating from 4.13.z to >=4.14.z.
// once users have migrated to an OCP version and have run this job at least once,
// this job is no longer needed. on OCP versions >=4.17 we can be certain that
Expand All @@ -220,15 +166,15 @@ func (c *AzurePathFixController) sync() error {
}
if len(removeConditionFns) > 0 {
if _, _, err := v1helpers.UpdateStatus(
context.TODO(),
ctx,
c.operatorClient,
removeConditionFns...,
); err != nil {
return err
}
}

_, err = gen.Get()
_, err = c.jobLister.Get(defaults.AzurePathFixJobName)
if errors.IsNotFound(err) {
return nil
} else if err != nil {
Expand All @@ -240,11 +186,13 @@ func (c *AzurePathFixController) sync() error {
GracePeriodSeconds: &gracePeriod,
PropagationPolicy: &propagationPolicy,
}
if err := gen.Delete(opts); err != nil {
if err := c.batchClient.Jobs(defaults.ImageRegistryOperatorNamespace).Delete(
ctx, defaults.AzurePathFixJobName, opts,
); err != nil {
return err
}
}
return err
return nil
}

func (c *AzurePathFixController) Run(stopCh <-chan struct{}) {
Expand Down
6 changes: 0 additions & 6 deletions pkg/operator/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,17 +201,11 @@ func RunOperator(ctx context.Context, kubeconfig *restclient.Config) error {
}

azurePathFixController, err := NewAzurePathFixController(
kubeconfig,
kubeClient.BatchV1(),
configOperatorClient,
kubeInformers.Batch().V1().Jobs(),
imageregistryInformers.Imageregistry().V1().Configs(),
configInformers.Config().V1().Infrastructures(),
kubeInformers.Core().V1().Secrets(),
configInformers.Config().V1().Proxies(),
kubeInformersForOpenShiftConfig.Core().V1().ConfigMaps(),
kubeInformers.Core().V1().Pods(),
featureGateAccessor,
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/resource/azurepathfixjob.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (gapfj *generatorAzurePathFixJob) GetNamespace() string {
}

func (gapfj *generatorAzurePathFixJob) GetName() string {
return "azure-path-fix"
return defaults.AzurePathFixJobName
}

func (gapfj *generatorAzurePathFixJob) expected() (runtime.Object, error) {
Expand Down

0 comments on commit d0356a0

Please sign in to comment.