Skip to content

Commit

Permalink
Update ingressconfig_controller to use CNO Apply logic with subcontro…
Browse files Browse the repository at this point in the history
…ller

"ingress_controller". It used to apply changes with deprecated
cluster-network-operator field manager, that was overridden and removed
by ApplyObject. Thus, labels applied to openshift-host-network namespace
were periodically removed.

Signed-off-by: Nadia Pinaeva <npinaeva@redhat.com>
  • Loading branch information
npinaeva committed Feb 7, 2024
1 parent 2667137 commit 215168d
Showing 1 changed file with 19 additions and 37 deletions.
56 changes: 19 additions & 37 deletions pkg/controller/ingressconfig/ingressconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,16 @@ import (
"time"

operv1 "github.com/openshift/api/operator/v1"
"github.com/openshift/cluster-network-operator/pkg/apply"
cnoclient "github.com/openshift/cluster-network-operator/pkg/client"
"github.com/openshift/cluster-network-operator/pkg/controller/statusmanager"
"github.com/openshift/cluster-network-operator/pkg/names"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"

crclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
Expand All @@ -35,14 +34,14 @@ var ManifestPath = "./bindata"

// Add creates a new ingressConfig controller and adds it to the Manager. The Manager will set fields on the Controller
// and Start it when the Manager is Started.
func Add(mgr manager.Manager, status *statusmanager.StatusManager, _ cnoclient.Client) error {
func Add(mgr manager.Manager, status *statusmanager.StatusManager, c cnoclient.Client) error {

return add(mgr, newIngressConfigReconciler(mgr.GetClient(), status))
return add(mgr, newIngressConfigReconciler(mgr, c, status))
}

// newIngressConfigReconciler returns a new reconcile.Reconciler
func newIngressConfigReconciler(client crclient.Client, status *statusmanager.StatusManager) *ReconcileIngressConfigs {
return &ReconcileIngressConfigs{client: client, status: status}
func newIngressConfigReconciler(mgr manager.Manager, client cnoclient.Client, status *statusmanager.StatusManager) *ReconcileIngressConfigs {
return &ReconcileIngressConfigs{mgr: mgr, client: client, status: status}
}

// add adds a new Controller to mgr with r as the reconcile.Reconciler
Expand All @@ -64,7 +63,8 @@ var _ reconcile.Reconciler = &ReconcileIngressConfigs{}
// ReconcileIngressConfigs watches for updates to ingress controller configuration
// and sets the network policy related labels on the openshift-host-network namespace
type ReconcileIngressConfigs struct {
client crclient.Client
client cnoclient.Client
mgr manager.Manager
status *statusmanager.StatusManager
}

Expand All @@ -84,7 +84,7 @@ func (r *ReconcileIngressConfigs) Reconcile(ctx context.Context, request reconci
}
log.Printf("Reconciling update to IngressController %s/%s\n", request.Namespace, request.Name)
ingressControllerConfig := &operv1.IngressController{TypeMeta: metav1.TypeMeta{APIVersion: operv1.GroupVersion.String(), Kind: "IngressController"}}
err := r.client.Get(ctx, request.NamespacedName, ingressControllerConfig)
err := r.mgr.GetClient().Get(ctx, request.NamespacedName, ingressControllerConfig)
if err != nil {
if apierrors.IsNotFound(err) {
log.Printf("Ingress Controller configuration %s was deleted", request.NamespacedName.String())
Expand All @@ -111,36 +111,18 @@ func (r *ReconcileIngressConfigs) Reconcile(ctx context.Context, request reconci

// setLabelsOnNamespace sets the labels specified on the target namespace using the client API
func (r *ReconcileIngressConfigs) updatePolicyGroupLabelOnNamespace(ctx context.Context, targetNamespace string, add bool) error {
var err error
namespace := &corev1.Namespace{TypeMeta: metav1.TypeMeta{APIVersion: corev1.SchemeGroupVersion.String(), Kind: "Namespace"}}
err = r.client.Get(ctx, types.NamespacedName{Name: targetNamespace}, namespace)
if err != nil {
// FIXME: abhat - this needs to be handled better. Currently we have no good way to tell
// the difference as to whether the error is a result of
// a) host-network namespace should have existed, but GET failed
// b) host-network namespace should not have existed in the first place,
// and therefore this code should ideally not even have been called.
// The right way to address this would be to not even spawn the ingress
// controller if we are running in the context of a third party plugin
if apierrors.IsNotFound(err) {
return nil
}
return err
}
newNamespace := namespace.DeepCopy()
existingLabels := newNamespace.GetLabels()
if existingLabels == nil {
existingLabels = map[string]string{}
setLabels := map[string]string{}

if add {
setLabels[names.PolicyGroupLabelIngress] = names.PolicyGroupLabelIngressValue
setLabels[names.PolicyGroupLabelLegacy] = names.PolicyGroupLabelLegacyValue
}
if !add {
delete(existingLabels, names.PolicyGroupLabelIngress)
delete(existingLabels, names.PolicyGroupLabelLegacy)
} else {
existingLabels[names.PolicyGroupLabelIngress] = names.PolicyGroupLabelIngressValue
existingLabels[names.PolicyGroupLabelLegacy] = names.PolicyGroupLabelLegacyValue
nsUpdate := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: targetNamespace,
Labels: setLabels,
},
}

newNamespace.SetLabels(existingLabels)

return r.client.Patch(context.TODO(), newNamespace, crclient.MergeFrom(namespace))
return apply.ApplyObject(ctx, r.client, nsUpdate, "ingress_controller")
}

0 comments on commit 215168d

Please sign in to comment.