diff --git a/bindata/allowlist/config/configmap.yaml b/bindata/allowlist/config/configmap.yaml deleted file mode 100644 index 557a6a792d..0000000000 --- a/bindata/allowlist/config/configmap.yaml +++ /dev/null @@ -1,37 +0,0 @@ -apiVersion: v1 -kind: ConfigMap -metadata: - name: cni-sysctl-allowlist - namespace: openshift-multus - annotations: - kubernetes.io/description: | - Sysctl allowlist for nodes - release.openshift.io/version: "{{.ReleaseVersion}}" -data: - # - # Safe sysctls - # ------------- - # add to this list only sysctls that - # * must not have any influence on any other pod on the node - # * must not allow to harm the node's health - # * must not allow to gain CPU or memory resources outside of the resource limit of the pod - # - allowlist.conf: |- - ^net.ipv4.conf.IFNAME.accept_ra$ - ^net.ipv4.conf.IFNAME.accept_redirects$ - ^net.ipv4.conf.IFNAME.accept_source_route$ - ^net.ipv4.conf.IFNAME.arp_accept$ - ^net.ipv4.conf.IFNAME.arp_notify$ - ^net.ipv4.conf.IFNAME.disable_policy$ - ^net.ipv4.conf.IFNAME.rp_filter$ - ^net.ipv4.conf.IFNAME.secure_redirects$ - ^net.ipv4.conf.IFNAME.send_redirects$ - ^net.ipv6.conf.IFNAME.accept_ra$ - ^net.ipv6.conf.IFNAME.accept_redirects$ - ^net.ipv6.conf.IFNAME.accept_source_route$ - ^net.ipv6.conf.IFNAME.arp_accept$ - ^net.ipv6.conf.IFNAME.arp_notify$ - ^net.ipv6.conf.IFNAME.disable_ipv6$ - ^net.ipv6.conf.IFNAME.disable_policy$ - ^net.ipv6.neigh.IFNAME.base_reachable_time_ms$ - ^net.ipv6.neigh.IFNAME.retrans_time_ms$ diff --git a/pkg/controller/allowlist/allowlist_controller.go b/pkg/controller/allowlist/allowlist_controller.go index ee05ce437e..ee215a3967 100644 --- a/pkg/controller/allowlist/allowlist_controller.go +++ b/pkg/controller/allowlist/allowlist_controller.go @@ -1,3 +1,5 @@ +// Package allowlist implements a Kubernetes controller that distributes CNI +// sysctl allowlist configuration to cluster nodes. package allowlist import ( @@ -17,8 +19,6 @@ import ( 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/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -36,21 +36,15 @@ import ( ) const ( - allowlistDsName = "cni-sysctl-allowlist-ds" - allowlistAnnotation = "app=cni-sysctl-allowlist-ds" - manifestDir = "../../bindata/allowlist/daemonset" - allowlistManifestDir = "../../bindata/network/multus/004-sysctl-configmap.yaml" + dsName = "cni-sysctl-allowlist-ds" + dsAnnotation = "app=cni-sysctl-allowlist-ds" + dsManifestDir = "../../bindata/allowlist/daemonset" + // Note: The default values come from default-cni-sysctl-allowlist which multus creates. + defaultCMManifest = "../../bindata/network/multus/004-sysctl-configmap.yaml" ) -func Add(mgr manager.Manager, status *statusmanager.StatusManager, c cnoclient.Client, _ featuregates.FeatureGate) error { - return add(mgr, newReconciler(mgr, status, c)) -} - -func newReconciler(mgr manager.Manager, status *statusmanager.StatusManager, c cnoclient.Client) *ReconcileAllowlist { - return &ReconcileAllowlist{client: c, scheme: mgr.GetScheme(), status: status} -} - -func add(mgr manager.Manager, r *ReconcileAllowlist) error { +func Add(mgr manager.Manager, status *statusmanager.StatusManager, client cnoclient.Client, _ featuregates.FeatureGate) error { + r := &ReconcileAllowlist{client: client, status: status} c, err := controller.New("allowlist-controller", mgr, controller.Options{Reconciler: r}) if err != nil { return err @@ -59,7 +53,7 @@ func add(mgr manager.Manager, r *ReconcileAllowlist) error { // watch for changes in all configmaps in our namespace cmInformer := v1coreinformers.NewConfigMapInformer( r.client.Default().Kubernetes(), - names.MULTUS_NAMESPACE, + names.MultusNamespace, 0, // don't resync cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) @@ -73,7 +67,8 @@ func add(mgr manager.Manager, r *ReconcileAllowlist) error { predicate.NewPredicateFuncs(func(object crclient.Object) bool { // Only care about cni-sysctl-allowlist, but also watching for default-cni-sysctl-allowlist // as a trigger for creating cni-sysctl-allowlist if it doesn't exist - return (strings.Contains(object.GetName(), names.ALLOWLIST_CONFIG_NAME)) + // NOTE: the cni-sysctl-allowlist is hardcoded in pkg/network/multus.go:91 + return (strings.Contains(object.GetName(), names.AllowlistConfigName)) }), }, }) @@ -83,14 +78,13 @@ var _ reconcile.Reconciler = &ReconcileAllowlist{} type ReconcileAllowlist struct { client cnoclient.Client - scheme *runtime.Scheme status *statusmanager.StatusManager } func (r *ReconcileAllowlist) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { defer utilruntime.HandleCrash(r.status.SetDegradedOnPanicAndCrash) - if exists, err := daemonsetConfigExists(ctx, r.client); !exists { - err = createObjects(ctx, r.client, allowlistManifestDir) + if exists, err := allowlistConfigMapExists(ctx, r.client); !exists { + err = createObjectsFrom(ctx, r.client, defaultCMManifest) if err != nil { klog.Errorf("Failed to create allowlist config map: %v", err) return reconcile.Result{}, err @@ -100,23 +94,27 @@ func (r *ReconcileAllowlist) Reconcile(ctx context.Context, request reconcile.Re return reconcile.Result{}, err } - if request.Name != names.ALLOWLIST_CONFIG_NAME { + if request.Name != names.AllowlistConfigName { return reconcile.Result{}, nil } klog.Infof("Reconcile allowlist for %s/%s", request.Namespace, request.Name) - configMap, err := getConfig(ctx, r.client, request.NamespacedName) + configMap, err := getConfigMap(ctx, r.client, request.NamespacedName) if err != nil { klog.Errorf("Failed to get config map: %v", err) return reconcile.Result{}, err } + // Deletion handling: If user deletes the ConfigMap, we do nothing. + // The allowlist file persists on nodes and pods continue working. + // The auto-create check above will recreate the ConfigMap on next reconcile. + // This prevents accidental deletion from breaking pod creation. // No action to be taken if user deletes the config map. The sysctl's will stay unmodified until config map is recreated if configMap == nil { return reconcile.Result{}, nil } - defer cleanup(ctx, r.client) + defer cleanupDaemonSet(ctx, r.client) // If daemonset still exists, delete it and reconcile again ds, err := getDaemonSet(ctx, r.client) @@ -129,7 +127,7 @@ func (r *ReconcileAllowlist) Reconcile(ctx context.Context, request reconcile.Re return reconcile.Result{}, errors.New("retrying") } - err = createObjects(ctx, r.client, manifestDir) + err = createObjectsFrom(ctx, r.client, dsManifestDir) if err != nil { klog.Errorf("Failed to create allowlist daemonset: %v", err) return reconcile.Result{}, err @@ -150,26 +148,24 @@ func (r *ReconcileAllowlist) Reconcile(ctx context.Context, request reconcile.Re return reconcile.Result{}, nil } -func createObjects(ctx context.Context, client cnoclient.Client, manifestDir string) error { +func createObjectsFrom(ctx context.Context, client cnoclient.Client, manifestPath string) error { data := render.MakeRenderData() data.Data["MultusImage"] = os.Getenv("MULTUS_IMAGE") - data.Data["CniSysctlAllowlist"] = names.ALLOWLIST_CONFIG_NAME + data.Data["CniSysctlAllowlist"] = names.AllowlistConfigName data.Data["ReleaseVersion"] = os.Getenv("RELEASE_VERSION") - manifests, err := render.RenderDir(manifestDir, &data) + manifests, err := render.RenderDir(manifestPath, &data) if err != nil { return err } for _, obj := range manifests { - - err = createObject(ctx, client, obj) - if err != nil { - return err + if err := client.Default().CRClient().Create(ctx, obj); err != nil { + return errors.Wrapf(err, "error creating %s %s/%s", obj.GroupVersionKind(), obj.GetNamespace(), obj.GetName()) } } return nil } -func getConfig(ctx context.Context, client cnoclient.Client, namespacedName types.NamespacedName) (*corev1.ConfigMap, error) { +func getConfigMap(ctx context.Context, client cnoclient.Client, namespacedName types.NamespacedName) (*corev1.ConfigMap, error) { configMap := &corev1.ConfigMap{} err := client.Default().CRClient().Get(ctx, namespacedName, configMap) if err != nil { @@ -181,14 +177,6 @@ func getConfig(ctx context.Context, client cnoclient.Client, namespacedName type return configMap, nil } -func createObject(ctx context.Context, client cnoclient.Client, obj *unstructured.Unstructured) error { - err := client.Default().CRClient().Create(ctx, obj) - if err != nil { - return errors.Wrapf(err, "error creating %s %s/%s", obj.GroupVersionKind(), obj.GetNamespace(), obj.GetName()) - } - return nil -} - func checkDsPodsReady(ctx context.Context, client cnoclient.Client) error { return wait.PollUntilContextTimeout(ctx, time.Second, time.Minute, false, func(ctx context.Context) (done bool, err error) { ds, err := getDaemonSet(ctx, client) @@ -199,8 +187,8 @@ func checkDsPodsReady(ctx context.Context, client cnoclient.Client) error { return false, fmt.Errorf("failed to get UID of daemon set") } - podList, err := client.Default().Kubernetes().CoreV1().Pods(names.MULTUS_NAMESPACE).List( - ctx, metav1.ListOptions{LabelSelector: allowlistAnnotation}) + podList, err := client.Default().Kubernetes().CoreV1().Pods(names.MultusNamespace).List( + ctx, metav1.ListOptions{LabelSelector: dsAnnotation}) if err != nil { return false, err } @@ -223,32 +211,24 @@ func checkDsPodsReady(ctx context.Context, client cnoclient.Client) error { }) } -func cleanup(ctx context.Context, client cnoclient.Client) { +func cleanupDaemonSet(ctx context.Context, client cnoclient.Client) { ds, err := getDaemonSet(ctx, client) if err != nil { klog.Errorf("Error looking up allowlist daemonset : %+v", err) return } if ds != nil { - err = deleteDaemonSet(ctx, client) + err := client.Default().Kubernetes().AppsV1().DaemonSets(names.MultusNamespace).Delete( + ctx, dsName, metav1.DeleteOptions{}) if err != nil { klog.Errorf("Error cleaning up allow list daemonset: %+v", err) } } } -func deleteDaemonSet(ctx context.Context, client cnoclient.Client) error { - err := client.Default().Kubernetes().AppsV1().DaemonSets(names.MULTUS_NAMESPACE).Delete( - ctx, allowlistDsName, metav1.DeleteOptions{}) - if err != nil { - return err - } - return nil -} - func getDaemonSet(ctx context.Context, client cnoclient.Client) (*appsv1.DaemonSet, error) { - ds, err := client.Default().Kubernetes().AppsV1().DaemonSets(names.MULTUS_NAMESPACE).Get( - ctx, allowlistDsName, metav1.GetOptions{}) + ds, err := client.Default().Kubernetes().AppsV1().DaemonSets(names.MultusNamespace).Get( + ctx, dsName, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { return nil, nil @@ -258,9 +238,9 @@ func getDaemonSet(ctx context.Context, client cnoclient.Client) (*appsv1.DaemonS return ds, nil } -func daemonsetConfigExists(ctx context.Context, client cnoclient.Client) (bool, error) { - cm, err := client.Default().Kubernetes().CoreV1().ConfigMaps(names.MULTUS_NAMESPACE).Get( - ctx, names.ALLOWLIST_CONFIG_NAME, metav1.GetOptions{}) +func allowlistConfigMapExists(ctx context.Context, client cnoclient.Client) (bool, error) { + cm, err := client.Default().Kubernetes().CoreV1().ConfigMaps(names.MultusNamespace).Get( + ctx, names.AllowlistConfigName, metav1.GetOptions{}) if err != nil { if apierrors.IsNotFound(err) { return false, nil diff --git a/pkg/names/names.go b/pkg/names/names.go index db0d7bf93f..c2b34451b9 100644 --- a/pkg/names/names.go +++ b/pkg/names/names.go @@ -27,13 +27,13 @@ const APPLIED_PREFIX = "applied-" // Should match 00_namespace.yaml const APPLIED_NAMESPACE = "openshift-network-operator" -// MULTUS_NAMESPACE is the namespace where applied configuration +// MultusNamespace is the namespace where applied configuration // configmaps are stored. // Should match 00_namespace.yaml -const MULTUS_NAMESPACE = "openshift-multus" +const MultusNamespace = "openshift-multus" -// ALLOWLIST_CONFIG_NAME is the name of the allowlist ConfigMap -const ALLOWLIST_CONFIG_NAME = "cni-sysctl-allowlist" +// AllowlistConfigName is the name of the allowlist ConfigMap +const AllowlistConfigName = "cni-sysctl-allowlist" // IgnoreObjectErrorAnnotation is an annotation we can set on objects // to signal to the reconciler that we don't care if they fail to create