diff --git a/Makefile b/Makefile index ad668536a0..e1000e5a7c 100644 --- a/Makefile +++ b/Makefile @@ -17,6 +17,7 @@ include $(addprefix ./vendor/github.com/openshift/build-machinery-go/make/, \ $(call add-bindata,backingresources,./pkg/operator/staticpod/controller/backingresource/manifests/...,bindata,bindata,./pkg/operator/staticpod/controller/backingresource/bindata/bindata.go) $(call add-bindata,installer,./pkg/operator/staticpod/controller/installer/manifests/...,bindata,bindata,./pkg/operator/staticpod/controller/installer/bindata/bindata.go) $(call add-bindata,staticpod,./pkg/operator/staticpod/controller/prune/manifests/...,bindata,bindata,./pkg/operator/staticpod/controller/prune/bindata/bindata.go) +$(call add-bindata,guard,./pkg/operator/staticpod/controller/guard/manifests/...,bindata,bindata,./pkg/operator/staticpod/controller/guard/bindata/bindata.go) $(call add-bindata,auditpolicies,./pkg/operator/apiserver/audit/manifests/...,bindata,bindata,./pkg/operator/apiserver/audit/bindata/bindata.go) $(call add-bindata,podnetworkconnectivitychecks,pkg/operator/connectivitycheckcontroller/manifests/...,bindata,bindata,pkg/operator/connectivitycheckcontroller/bindata/bindata.go) diff --git a/pkg/operator/staticpod/controller/guard/bindata/bindata.go b/pkg/operator/staticpod/controller/guard/bindata/bindata.go new file mode 100644 index 0000000000..b346d1ec9a --- /dev/null +++ b/pkg/operator/staticpod/controller/guard/bindata/bindata.go @@ -0,0 +1,313 @@ +// Code generated for package bindata by go-bindata DO NOT EDIT. (@generated) +// sources: +// pkg/operator/staticpod/controller/guard/manifests/guard-pod.yaml +// pkg/operator/staticpod/controller/guard/manifests/pdb.yaml +package bindata + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + "time" +) + +type asset struct { + bytes []byte + info os.FileInfo +} + +type bindataFileInfo struct { + name string + size int64 + mode os.FileMode + modTime time.Time +} + +// Name return file name +func (fi bindataFileInfo) Name() string { + return fi.name +} + +// Size return file size +func (fi bindataFileInfo) Size() int64 { + return fi.size +} + +// Mode return file mode +func (fi bindataFileInfo) Mode() os.FileMode { + return fi.mode +} + +// Mode return file modify time +func (fi bindataFileInfo) ModTime() time.Time { + return fi.modTime +} + +// IsDir return file whether a directory +func (fi bindataFileInfo) IsDir() bool { + return fi.mode&os.ModeDir != 0 +} + +// Sys return file is sys mode +func (fi bindataFileInfo) Sys() interface{} { + return nil +} + +var _pkgOperatorStaticpodControllerGuardManifestsGuardPodYaml = []byte(`apiVersion: v1 +kind: Pod +metadata: + namespace: # Value set by operator + name: # Value set by operator + labels: + app: guard + ownerReferences: # Value set by operator +spec: + affinity: # Value set by operator + priorityClassName: "system-cluster-critical" + terminationGracePeriodSeconds: 3 + tolerations: + - key: node-role.kubernetes.io/master + effect: NoSchedule + operator: Exists + - key: node.kubernetes.io/not-ready + effect: NoExecute + operator: Exists + - key: node.kubernetes.io/unreachable + effect: NoExecute + operator: Exists + - key: node-role.kubernetes.io/etcd + operator: Exists + effect: NoSchedule + containers: + - name: guard + image: # Value set by operator + imagePullPolicy: IfNotPresent + terminationMessagePolicy: FallbackToLogsOnError + command: + - /bin/bash + args: + - -c + - | + # properly handle TERM and exit as soon as it is signaled + set -euo pipefail + trap 'jobs -p | xargs -r kill; exit 0' TERM + sleep infinity & wait + readinessProbe: + failureThreshold: 3 + httpGet: + host: # Value set by operator + path: healthz + port: # Value set by operator + scheme: HTTPS + periodSeconds: 5 + successThreshold: 1 + timeoutSeconds: 5 + resources: + requests: + cpu: 10m + memory: 5Mi +`) + +func pkgOperatorStaticpodControllerGuardManifestsGuardPodYamlBytes() ([]byte, error) { + return _pkgOperatorStaticpodControllerGuardManifestsGuardPodYaml, nil +} + +func pkgOperatorStaticpodControllerGuardManifestsGuardPodYaml() (*asset, error) { + bytes, err := pkgOperatorStaticpodControllerGuardManifestsGuardPodYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "pkg/operator/staticpod/controller/guard/manifests/guard-pod.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var _pkgOperatorStaticpodControllerGuardManifestsPdbYaml = []byte(`apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: # Value set by operator + namespace: # Value set by operator +spec: + minAvailable: 0 # Value set by operator + selector: + matchLabels: + app: guard +`) + +func pkgOperatorStaticpodControllerGuardManifestsPdbYamlBytes() ([]byte, error) { + return _pkgOperatorStaticpodControllerGuardManifestsPdbYaml, nil +} + +func pkgOperatorStaticpodControllerGuardManifestsPdbYaml() (*asset, error) { + bytes, err := pkgOperatorStaticpodControllerGuardManifestsPdbYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "pkg/operator/staticpod/controller/guard/manifests/pdb.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +// Asset loads and returns the asset for the given name. +// It returns an error if the asset could not be found or +// could not be loaded. +func Asset(name string) ([]byte, error) { + cannonicalName := strings.Replace(name, "\\", "/", -1) + if f, ok := _bindata[cannonicalName]; ok { + a, err := f() + if err != nil { + return nil, fmt.Errorf("Asset %s can't read by error: %v", name, err) + } + return a.bytes, nil + } + return nil, fmt.Errorf("Asset %s not found", name) +} + +// MustAsset is like Asset but panics when Asset would return an error. +// It simplifies safe initialization of global variables. +func MustAsset(name string) []byte { + a, err := Asset(name) + if err != nil { + panic("asset: Asset(" + name + "): " + err.Error()) + } + + return a +} + +// AssetInfo loads and returns the asset info for the given name. +// It returns an error if the asset could not be found or +// could not be loaded. +func AssetInfo(name string) (os.FileInfo, error) { + cannonicalName := strings.Replace(name, "\\", "/", -1) + if f, ok := _bindata[cannonicalName]; ok { + a, err := f() + if err != nil { + return nil, fmt.Errorf("AssetInfo %s can't read by error: %v", name, err) + } + return a.info, nil + } + return nil, fmt.Errorf("AssetInfo %s not found", name) +} + +// AssetNames returns the names of the assets. +func AssetNames() []string { + names := make([]string, 0, len(_bindata)) + for name := range _bindata { + names = append(names, name) + } + return names +} + +// _bindata is a table, holding each asset generator, mapped to its name. +var _bindata = map[string]func() (*asset, error){ + "pkg/operator/staticpod/controller/guard/manifests/guard-pod.yaml": pkgOperatorStaticpodControllerGuardManifestsGuardPodYaml, + "pkg/operator/staticpod/controller/guard/manifests/pdb.yaml": pkgOperatorStaticpodControllerGuardManifestsPdbYaml, +} + +// AssetDir returns the file names below a certain +// directory embedded in the file by go-bindata. +// For example if you run go-bindata on data/... and data contains the +// following hierarchy: +// data/ +// foo.txt +// img/ +// a.png +// b.png +// then AssetDir("data") would return []string{"foo.txt", "img"} +// AssetDir("data/img") would return []string{"a.png", "b.png"} +// AssetDir("foo.txt") and AssetDir("notexist") would return an error +// AssetDir("") will return []string{"data"}. +func AssetDir(name string) ([]string, error) { + node := _bintree + if len(name) != 0 { + cannonicalName := strings.Replace(name, "\\", "/", -1) + pathList := strings.Split(cannonicalName, "/") + for _, p := range pathList { + node = node.Children[p] + if node == nil { + return nil, fmt.Errorf("Asset %s not found", name) + } + } + } + if node.Func != nil { + return nil, fmt.Errorf("Asset %s not found", name) + } + rv := make([]string, 0, len(node.Children)) + for childName := range node.Children { + rv = append(rv, childName) + } + return rv, nil +} + +type bintree struct { + Func func() (*asset, error) + Children map[string]*bintree +} + +var _bintree = &bintree{nil, map[string]*bintree{ + "pkg": {nil, map[string]*bintree{ + "operator": {nil, map[string]*bintree{ + "staticpod": {nil, map[string]*bintree{ + "controller": {nil, map[string]*bintree{ + "guard": {nil, map[string]*bintree{ + "manifests": {nil, map[string]*bintree{ + "guard-pod.yaml": {pkgOperatorStaticpodControllerGuardManifestsGuardPodYaml, map[string]*bintree{}}, + "pdb.yaml": {pkgOperatorStaticpodControllerGuardManifestsPdbYaml, map[string]*bintree{}}, + }}, + }}, + }}, + }}, + }}, + }}, +}} + +// RestoreAsset restores an asset under the given directory +func RestoreAsset(dir, name string) error { + data, err := Asset(name) + if err != nil { + return err + } + info, err := AssetInfo(name) + if err != nil { + return err + } + err = os.MkdirAll(_filePath(dir, filepath.Dir(name)), os.FileMode(0755)) + if err != nil { + return err + } + err = ioutil.WriteFile(_filePath(dir, name), data, info.Mode()) + if err != nil { + return err + } + err = os.Chtimes(_filePath(dir, name), info.ModTime(), info.ModTime()) + if err != nil { + return err + } + return nil +} + +// RestoreAssets restores an asset under the given directory recursively +func RestoreAssets(dir, name string) error { + children, err := AssetDir(name) + // File + if err != nil { + return RestoreAsset(dir, name) + } + // Dir + for _, child := range children { + err = RestoreAssets(dir, filepath.Join(name, child)) + if err != nil { + return err + } + } + return nil +} + +func _filePath(dir, name string) string { + cannonicalName := strings.Replace(name, "\\", "/", -1) + return filepath.Join(append([]string{dir}, strings.Split(cannonicalName, "/")...)...) +} diff --git a/pkg/operator/staticpod/controller/guard/guard_controller.go b/pkg/operator/staticpod/controller/guard/guard_controller.go new file mode 100644 index 0000000000..ca47c78d81 --- /dev/null +++ b/pkg/operator/staticpod/controller/guard/guard_controller.go @@ -0,0 +1,291 @@ +package guard + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strconv" + + 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/labels" + "k8s.io/apimachinery/pkg/selection" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/informers" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + policyclientv1 "k8s.io/client-go/kubernetes/typed/policy/v1" + corelisterv1 "k8s.io/client-go/listers/core/v1" + policylisterv1 "k8s.io/client-go/listers/policy/v1" + "k8s.io/klog/v2" + + configv1 "github.com/openshift/api/config/v1" + configv1listers "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/resourceapply" + "github.com/openshift/library-go/pkg/operator/resource/resourceread" + "github.com/openshift/library-go/pkg/operator/staticpod/controller/guard/bindata" + operatorv1helpers "github.com/openshift/library-go/pkg/operator/v1helpers" +) + +// GuardController is a controller that watches amount of static pods on master nodes and +// renders guard pods with a pdb to keep maxUnavailable to be at most 1 +type GuardController struct { + targetNamespace, podResourcePrefix string + operatorName string + readyzPort string + + nodeLister corelisterv1.NodeLister + podLister corelisterv1.PodLister + podGetter corev1client.PodsGetter + pdbGetter policyclientv1.PodDisruptionBudgetsGetter + pdbLister policylisterv1.PodDisruptionBudgetLister + + // installerPodImageFn returns the image name for the installer pod + installerPodImageFn func() string + createConditionalFunc func() (bool, error) +} + +func NewGuardController( + targetNamespace, podResourcePrefix string, + operatorName string, + readyzPort string, + kubeInformersForTargetNamespace informers.SharedInformerFactory, + kubeInformersClusterScoped informers.SharedInformerFactory, + operatorClient operatorv1helpers.StaticPodOperatorClient, + podGetter corev1client.PodsGetter, + pdbGetter policyclientv1.PodDisruptionBudgetsGetter, + eventRecorder events.Recorder, + createConditionalFunc func() (bool, error), +) factory.Controller { + c := &GuardController{ + targetNamespace: targetNamespace, + podResourcePrefix: podResourcePrefix, + operatorName: operatorName, + readyzPort: readyzPort, + nodeLister: kubeInformersClusterScoped.Core().V1().Nodes().Lister(), + podLister: kubeInformersForTargetNamespace.Core().V1().Pods().Lister(), + podGetter: podGetter, + pdbGetter: pdbGetter, + pdbLister: kubeInformersForTargetNamespace.Policy().V1().PodDisruptionBudgets().Lister(), + installerPodImageFn: getInstallerPodImageFromEnv, + createConditionalFunc: createConditionalFunc, + } + + return factory.New().WithInformers( + kubeInformersForTargetNamespace.Core().V1().Pods().Informer(), + kubeInformersClusterScoped.Core().V1().Nodes().Informer(), + ).WithSync(c.sync).WithSyncDegradedOnError(operatorClient).ToController("GuardController", eventRecorder) +} + +func getInstallerPodImageFromEnv() string { + return os.Getenv("OPERATOR_IMAGE") +} + +func getGuardPodName(prefix, nodeName string) string { + return fmt.Sprintf("%s-guard-%s", prefix, nodeName) +} + +func getGuardPDBName(prefix string) string { + return fmt.Sprintf("%s-guard-pdb", prefix) +} + +func nodeConditionFinder(status *corev1.NodeStatus, condType corev1.NodeConditionType) *corev1.NodeCondition { + for i := range status.Conditions { + if status.Conditions[i].Type == condType { + return &status.Conditions[i] + } + } + + return nil +} + +func (c *GuardController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + klog.V(5).Info("Syncing guards") + + if c.createConditionalFunc == nil { + return fmt.Errorf("create conditional not set") + } + + shouldCreate, err := c.createConditionalFunc() + if err != nil { + return fmt.Errorf("create conditional returns an error: %v", err) + } + + errs := []error{} + if !shouldCreate { + pdb := resourceread.ReadPodDisruptionBudgetV1OrDie(bindata.MustAsset(filepath.Join("pkg/operator/staticpod/controller/guard", "manifests/pdb.yaml"))) + pdb.ObjectMeta.Name = getGuardPDBName(c.podResourcePrefix) + pdb.ObjectMeta.Namespace = c.targetNamespace + + // List the pdb from the cache in case it does not exist and there's nothing to delete + // so no Delete request is executed. + pdbs, err := c.pdbLister.PodDisruptionBudgets(c.targetNamespace).List(labels.Everything()) + if err != nil { + klog.Errorf("Unable to list PodDisruptionBudgets: %v", err) + return err + } + + for _, pdbItem := range pdbs { + if pdbItem.Name == pdb.Name { + _, _, err := resourceapply.DeletePodDisruptionBudget(ctx, c.pdbGetter, syncCtx.Recorder(), pdb) + if err != nil { + klog.Errorf("Unable to delete PodDisruptionBudget: %v", err) + errs = append(errs, err) + } + break + } + } + + pods, err := c.podLister.Pods(c.targetNamespace).List(labels.SelectorFromSet(labels.Set{"app": "guard"})) + if err != nil { + errs = append(errs, err) + } else { + for _, pod := range pods { + _, _, err = resourceapply.DeletePod(ctx, c.podGetter, syncCtx.Recorder(), pod) + if err != nil { + klog.Errorf("Unable to delete Pod: %v", err) + errs = append(errs, err) + } + } + } + } else { + selector, err := labels.NewRequirement("node-role.kubernetes.io/master", selection.Equals, []string{""}) + if err != nil { + panic(err) + } + nodes, err := c.nodeLister.List(labels.NewSelector().Add(*selector)) + if err != nil { + return err + } + + pods, err := c.podLister.Pods(c.targetNamespace).List(labels.SelectorFromSet(labels.Set{"app": c.podResourcePrefix})) + if err != nil { + return err + } + + klog.V(5).Infof("Rendering guard pdb") + + pdb := resourceread.ReadPodDisruptionBudgetV1OrDie(bindata.MustAsset(filepath.Join("pkg/operator/staticpod/controller/guard", "manifests/pdb.yaml"))) + pdb.ObjectMeta.Name = getGuardPDBName(c.podResourcePrefix) + pdb.ObjectMeta.Namespace = c.targetNamespace + if len(nodes) > 1 { + minAvailable := intstr.FromInt(len(nodes) - 1) + pdb.Spec.MinAvailable = &minAvailable + } + + // List the pdb from the cache in case it exists and there's nothing to update + // so no Get request is executed. + pdbs, err := c.pdbLister.PodDisruptionBudgets(c.targetNamespace).List(labels.Everything()) + if err != nil { + klog.Errorf("Unable to list PodDisruptionBudgets: %v", err) + return err + } + + for _, pdbItem := range pdbs { + if pdbItem.Name == pdb.Name { + if pdbItem.Spec.MinAvailable != pdb.Spec.MinAvailable { + _, _, err = resourceapply.ApplyPodDisruptionBudget(ctx, c.pdbGetter, syncCtx.Recorder(), pdb) + if err != nil { + klog.Errorf("Unable to apply PodDisruptionBudget changes: %v", err) + return err + } + } + break + } + } + + operands := map[string]*corev1.Pod{} + for _, pod := range pods { + operands[pod.Spec.NodeName] = pod + } + + for _, node := range nodes { + if _, exists := operands[node.Name]; !exists { + // If the operand does not exist and the node is not ready, wait until the node becomes ready + nodeReadyCondition := nodeConditionFinder(&node.Status, corev1.NodeReady) + // If a "Ready" condition is not found, that node should be deemed as not Ready by default. + if nodeReadyCondition == nil || nodeReadyCondition.Status != corev1.ConditionTrue { + klog.Infof("Node %v not ready, skipping reconciling the guard pod", node.Name) + continue + } + + klog.Errorf("Missing operand on node %v", node.Name) + errs = append(errs, fmt.Errorf("Missing operand on node %v", node.Name)) + continue + } + + if operands[node.Name].Status.PodIP == "" { + klog.Errorf("Missing PodIP in operand %v on node %v", operands[node.Name].Name, node.Name) + errs = append(errs, fmt.Errorf("Missing PodIP in operand %v on node %v", operands[node.Name].Name, node.Name)) + continue + } + + klog.V(5).Infof("Rendering guard pod for operand %v on node %v", operands[node.Name].Name, node.Name) + + pod := resourceread.ReadPodV1OrDie(bindata.MustAsset(filepath.Join("pkg/operator/staticpod/controller/guard", "manifests/guard-pod.yaml"))) + + pod.ObjectMeta.Name = getGuardPodName(c.podResourcePrefix, node.Name) + pod.ObjectMeta.Namespace = c.targetNamespace + pod.Spec.NodeName = node.Name + pod.Spec.Containers[0].Image = c.installerPodImageFn() + pod.Spec.Containers[0].ReadinessProbe.HTTPGet.Host = operands[node.Name].Status.PodIP + // The readyz port as string type is expected to be convertible into int!!! + readyzPort, err := strconv.Atoi(c.readyzPort) + if err != nil { + panic(err) + } + pod.Spec.Containers[0].ReadinessProbe.HTTPGet.Port = intstr.FromInt(readyzPort) + + actual, err := c.podGetter.Pods(pod.Namespace).Get(ctx, pod.Name, metav1.GetOptions{}) + if err == nil { + // Delete the pod so it can be re-created. ApplyPod only updates the metadata part of the manifests, ignores the rest + delete := false + if actual.Spec.Containers[0].Image != pod.Spec.Containers[0].Image { + klog.V(5).Infof("Guard Image changed, deleting %v so the guard can be re-created", pod.Name) + delete = true + } + if actual.Spec.Containers[0].ReadinessProbe.HTTPGet.Host != pod.Spec.Containers[0].ReadinessProbe.HTTPGet.Host { + klog.V(5).Infof("Operand PodIP changed, deleting %v so the guard can be re-created", pod.Name) + delete = true + } + if delete { + _, _, err = resourceapply.DeletePod(ctx, c.podGetter, syncCtx.Recorder(), pod) + if err != nil { + klog.Errorf("Unable to delete Pod for immidiate re-creation: %v", err) + errs = append(errs, fmt.Errorf("Unable to delete Pod for immidiate re-creation: %v", err)) + continue + } + } + } else if !apierrors.IsNotFound(err) { + errs = append(errs, err) + continue + } + + _, _, err = resourceapply.ApplyPod(ctx, c.podGetter, syncCtx.Recorder(), pod) + if err != nil { + klog.Errorf("Unable to apply pod %v changes: %v", pod.Name, err) + errs = append(errs, fmt.Errorf("Unable to apply pod %v changes: %v", pod.Name, err)) + } + } + } + + return utilerrors.NewAggregate(errs) +} + +func IsSNOCheckFnc(infraLister configv1listers.InfrastructureLister) func() (bool, error) { + return func() (bool, error) { + infraData, err := infraLister.Get("cluster") + if err != nil { + return false, fmt.Errorf("Unable to list infrastructures.config.openshift.io/cluster object, unable to determine topology mode") + } + if infraData.Status.ControlPlaneTopology == "" { + return false, fmt.Errorf("ControlPlaneTopology was not set, unable to determine topology mode") + } + + return infraData.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode, nil + } +} diff --git a/pkg/operator/staticpod/controller/guard/guard_controller_test.go b/pkg/operator/staticpod/controller/guard/guard_controller_test.go new file mode 100644 index 0000000000..5d89107a05 --- /dev/null +++ b/pkg/operator/staticpod/controller/guard/guard_controller_test.go @@ -0,0 +1,448 @@ +package guard + +import ( + "context" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" + "k8s.io/client-go/util/workqueue" + + configv1 "github.com/openshift/api/config/v1" + configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/library-go/pkg/operator/events" +) + +type FakeInfrastructureLister struct { + InfrastructureLister_ configlistersv1.InfrastructureLister +} + +func (l FakeInfrastructureLister) Get(name string) (*configv1.Infrastructure, error) { + return l.InfrastructureLister_.Get(name) +} + +func (l FakeInfrastructureLister) List(selector labels.Selector) (ret []*configv1.Infrastructure, err error) { + return l.InfrastructureLister_.List(selector) +} + +func TestIsSNOCheckFnc(t *testing.T) { + tests := []struct { + name string + infraObject *configv1.Infrastructure + result bool + err bool + }{ + { + name: "Missing Infrastructure status", + infraObject: &configv1.Infrastructure{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{}, + }, + result: false, + err: true, + }, + { + name: "Missing ControlPlaneTopology", + infraObject: &configv1.Infrastructure{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + ControlPlaneTopology: "", + }, + }, + result: false, + err: true, + }, + { + name: "ControlPlaneTopology not SingleReplicaTopologyMode", + infraObject: &configv1.Infrastructure{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + ControlPlaneTopology: configv1.HighlyAvailableTopologyMode, + }, + }, + result: false, + }, + { + name: "ControlPlaneTopology is SingleReplicaTopologyMode", + infraObject: &configv1.Infrastructure{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + ControlPlaneTopology: configv1.SingleReplicaTopologyMode, + }, + }, + result: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if err := indexer.Add(test.infraObject); err != nil { + t.Fatal(err.Error()) + } + lister := FakeInfrastructureLister{ + InfrastructureLister_: configlistersv1.NewInfrastructureLister(indexer), + } + + conditionalFunction := IsSNOCheckFnc(lister) + result, err := conditionalFunction() + if test.err { + if err == nil { + t.Errorf("%s: expected error, got none", test.name) + } + } else { + if err != nil { + t.Errorf("%s: unexpected error: %v", test.name, err) + } else if result != test.result { + t.Errorf("%s: expected %v, got %v", test.name, test.result, result) + } + } + }) + } +} + +func fakeMasterNode(name string) *corev1.Node { + n := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + "node-role.kubernetes.io/master": "", + }, + }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + { + Type: corev1.NodeReady, + Status: corev1.ConditionTrue, + }, + }, + }, + } + + return n +} + +type FakeSyncContext struct { + recorder events.Recorder +} + +func (f FakeSyncContext) Queue() workqueue.RateLimitingInterface { + return nil +} + +func (f FakeSyncContext) QueueKey() string { + return "" +} + +func (f FakeSyncContext) Recorder() events.Recorder { + return f.recorder +} + +// render a guarding pod +func TestRenderGuardPod(t *testing.T) { + tests := []struct { + name string + infraObject *configv1.Infrastructure + errString string + err bool + operandPod *corev1.Pod + guardExists bool + guardPod *corev1.Pod + }{ + { + name: "Operand pod missing", + infraObject: &configv1.Infrastructure{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + ControlPlaneTopology: configv1.SingleReplicaTopologyMode, + }, + }, + errString: "Missing operand on node master1", + err: true, + operandPod: nil, + }, + { + name: "Operand pod missing .Status.PodIP", + infraObject: &configv1.Infrastructure{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + ControlPlaneTopology: configv1.SingleReplicaTopologyMode, + }, + }, + errString: "Missing PodIP in operand operand1 on node master1", + err: true, + operandPod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "operand1", + Namespace: "test", + Labels: map[string]string{"app": "operand"}, + }, + Spec: corev1.PodSpec{ + NodeName: "master1", + }, + Status: corev1.PodStatus{}, + }, + }, + { + name: "Operand guard pod created", + infraObject: &configv1.Infrastructure{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + ControlPlaneTopology: configv1.SingleReplicaTopologyMode, + }, + }, + operandPod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "operand1", + Namespace: "test", + Labels: map[string]string{"app": "operand"}, + }, + Spec: corev1.PodSpec{ + NodeName: "master1", + }, + Status: corev1.PodStatus{ + PodIP: "1.1.1.1", + }, + }, + guardExists: true, + }, + { + name: "Operand guard pod deleted", + infraObject: &configv1.Infrastructure{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + ControlPlaneTopology: configv1.HighlyAvailableTopologyMode, + }, + }, + operandPod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "operand1", + Namespace: "test", + Labels: map[string]string{"app": "operand"}, + }, + Spec: corev1.PodSpec{ + NodeName: "master1", + }, + Status: corev1.PodStatus{ + PodIP: "1.1.1.1", + }, + }, + guardPod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: getGuardPodName("operand", "master1"), + Namespace: "test", + Labels: map[string]string{"app": "guard"}, + }, + Spec: corev1.PodSpec{ + NodeName: "master1", + }, + Status: corev1.PodStatus{ + PodIP: "1.1.1.1", + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if err := indexer.Add(test.infraObject); err != nil { + t.Fatal(err.Error()) + } + lister := FakeInfrastructureLister{ + InfrastructureLister_: configlistersv1.NewInfrastructureLister(indexer), + } + + kubeClient := fake.NewSimpleClientset(fakeMasterNode("master1")) + if test.operandPod != nil { + kubeClient.Tracker().Add(test.operandPod) + } + if test.guardPod != nil { + kubeClient.Tracker().Add(test.guardPod) + } + kubeInformers := informers.NewSharedInformerFactoryWithOptions(kubeClient, 1*time.Minute) + eventRecorder := events.NewRecorder(kubeClient.CoreV1().Events("test"), "test-operator", &corev1.ObjectReference{}) + + ctrl := &GuardController{ + targetNamespace: "test", + podResourcePrefix: "operand", + operatorName: "operator", + readyzPort: "99999", + nodeLister: kubeInformers.Core().V1().Nodes().Lister(), + podLister: kubeInformers.Core().V1().Pods().Lister(), + podGetter: kubeClient.CoreV1(), + pdbGetter: kubeClient.PolicyV1(), + pdbLister: kubeInformers.Policy().V1().PodDisruptionBudgets().Lister(), + installerPodImageFn: getInstallerPodImageFromEnv, + createConditionalFunc: IsSNOCheckFnc(lister), + } + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + kubeInformers.Start(ctx.Done()) + kubeInformers.WaitForCacheSync(ctx.Done()) + + err := ctrl.sync(ctx, FakeSyncContext{recorder: eventRecorder}) + if test.err { + if test.errString != err.Error() { + t.Errorf("%s: expected error message %q, got %q", test.name, test.errString, err) + } + } else { + if test.guardExists { + p, err := kubeClient.CoreV1().Pods("test").Get(ctx, getGuardPodName("operand", "master1"), metav1.GetOptions{}) + if err != nil { + t.Errorf("%s: unexpected error: %v", test.name, err) + } else { + probe := p.Spec.Containers[0].ReadinessProbe.HTTPGet + if probe == nil { + t.Errorf("%s: missing ReadinessProbe in the guard", test.name) + } + if probe.Host != test.operandPod.Status.PodIP { + t.Errorf("%s: expected %q host in ReadinessProbe in the guard, got %q instead", test.name, test.operandPod.Status.PodIP, probe.Host) + } + + if probe.Port.IntValue() != 99999 { + t.Errorf("%s: unexpected port in ReadinessProbe in the guard, expected 99999, got %v instead", test.name, probe.Port.IntValue()) + } + } + } else { + _, err := kubeClient.CoreV1().Pods("test").Get(ctx, getGuardPodName("operand", "master1"), metav1.GetOptions{}) + if !apierrors.IsNotFound(err) { + t.Errorf("%s: expected 'pods \"%v\" not found' error, got %q instead", test.name, getGuardPodName("operand", "master1"), err) + } + } + } + }) + } +} + +// change a guard pod based on a change of an operand ip address (to update the readiness probe) +func TestRenderGuardPodPortChanged(t *testing.T) { + infraObject := &configv1.Infrastructure{ + ObjectMeta: v1.ObjectMeta{ + Name: "cluster", + }, + Status: configv1.InfrastructureStatus{ + ControlPlaneTopology: configv1.SingleReplicaTopologyMode, + }, + } + operandPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "operand1", + Namespace: "test", + Labels: map[string]string{"app": "operand"}, + }, + Spec: corev1.PodSpec{ + NodeName: "master1", + }, + Status: corev1.PodStatus{ + PodIP: "2.2.2.2", + }, + } + guardPod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: getGuardPodName("operand", "master1"), + Namespace: "test", + Labels: map[string]string{"app": "guard"}, + }, + Spec: corev1.PodSpec{ + NodeName: "master1", + Containers: []corev1.Container{ + { + Image: "", + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Host: "1.1.1.1", + Port: intstr.FromInt(99999), + }, + }, + }, + }, + }, + }, + Status: corev1.PodStatus{ + PodIP: "1.1.1.1", + }, + } + + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + if err := indexer.Add(infraObject); err != nil { + t.Fatal(err.Error()) + } + lister := FakeInfrastructureLister{ + InfrastructureLister_: configlistersv1.NewInfrastructureLister(indexer), + } + + kubeClient := fake.NewSimpleClientset(fakeMasterNode("master1"), operandPod, guardPod) + kubeInformers := informers.NewSharedInformerFactoryWithOptions(kubeClient, 1*time.Minute) + eventRecorder := events.NewRecorder(kubeClient.CoreV1().Events("test"), "test-operator", &corev1.ObjectReference{}) + + ctrl := &GuardController{ + targetNamespace: "test", + podResourcePrefix: "operand", + operatorName: "operator", + readyzPort: "99999", + nodeLister: kubeInformers.Core().V1().Nodes().Lister(), + podLister: kubeInformers.Core().V1().Pods().Lister(), + podGetter: kubeClient.CoreV1(), + pdbGetter: kubeClient.PolicyV1(), + pdbLister: kubeInformers.Policy().V1().PodDisruptionBudgets().Lister(), + installerPodImageFn: getInstallerPodImageFromEnv, + createConditionalFunc: IsSNOCheckFnc(lister), + } + + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + + kubeInformers.Start(ctx.Done()) + kubeInformers.WaitForCacheSync(ctx.Done()) + + // expected to pass + if err := ctrl.sync(ctx, FakeSyncContext{recorder: eventRecorder}); err != nil { + t.Fatal(err.Error()) + } + + // check the probe.Host is the same as the operand ip address + p, err := kubeClient.CoreV1().Pods("test").Get(ctx, getGuardPodName("operand", "master1"), metav1.GetOptions{}) + if err != nil { + t.Errorf("unexpected error: %v", err) + } else { + probe := p.Spec.Containers[0].ReadinessProbe.HTTPGet + if probe == nil { + t.Errorf("missing ReadinessProbe in the guard") + } + if probe.Host != operandPod.Status.PodIP { + t.Errorf("expected %q host in ReadinessProbe in the guard, got %q instead", operandPod.Status.PodIP, probe.Host) + } + + if probe.Port.IntValue() != 99999 { + t.Errorf("unexpected port in ReadinessProbe in the guard, expected 99999, got %v instead", probe.Port.IntValue()) + } + } +} diff --git a/pkg/operator/staticpod/controller/guard/manifests/guard-pod.yaml b/pkg/operator/staticpod/controller/guard/manifests/guard-pod.yaml new file mode 100644 index 0000000000..bc05d471d8 --- /dev/null +++ b/pkg/operator/staticpod/controller/guard/manifests/guard-pod.yaml @@ -0,0 +1,53 @@ +apiVersion: v1 +kind: Pod +metadata: + namespace: # Value set by operator + name: # Value set by operator + labels: + app: guard + ownerReferences: # Value set by operator +spec: + affinity: # Value set by operator + priorityClassName: "system-cluster-critical" + terminationGracePeriodSeconds: 3 + tolerations: + - key: node-role.kubernetes.io/master + effect: NoSchedule + operator: Exists + - key: node.kubernetes.io/not-ready + effect: NoExecute + operator: Exists + - key: node.kubernetes.io/unreachable + effect: NoExecute + operator: Exists + - key: node-role.kubernetes.io/etcd + operator: Exists + effect: NoSchedule + containers: + - name: guard + image: # Value set by operator + imagePullPolicy: IfNotPresent + terminationMessagePolicy: FallbackToLogsOnError + command: + - /bin/bash + args: + - -c + - | + # properly handle TERM and exit as soon as it is signaled + set -euo pipefail + trap 'jobs -p | xargs -r kill; exit 0' TERM + sleep infinity & wait + readinessProbe: + failureThreshold: 3 + httpGet: + host: # Value set by operator + path: healthz + port: # Value set by operator + scheme: HTTPS + periodSeconds: 5 + successThreshold: 1 + timeoutSeconds: 5 + resources: + requests: + cpu: 10m + memory: 5Mi diff --git a/pkg/operator/staticpod/controller/guard/manifests/pdb.yaml b/pkg/operator/staticpod/controller/guard/manifests/pdb.yaml new file mode 100644 index 0000000000..9ba7971d0c --- /dev/null +++ b/pkg/operator/staticpod/controller/guard/manifests/pdb.yaml @@ -0,0 +1,10 @@ +apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: # Value set by operator + namespace: # Value set by operator +spec: + minAvailable: 0 # Value set by operator + selector: + matchLabels: + app: guard diff --git a/pkg/operator/staticpod/controllers.go b/pkg/operator/staticpod/controllers.go index 112fd0d366..00025e3758 100644 --- a/pkg/operator/staticpod/controllers.go +++ b/pkg/operator/staticpod/controllers.go @@ -11,6 +11,7 @@ import ( "github.com/openshift/library-go/pkg/operator/resource/resourceapply" "github.com/openshift/library-go/pkg/operator/revisioncontroller" "github.com/openshift/library-go/pkg/operator/staticpod/controller/backingresource" + "github.com/openshift/library-go/pkg/operator/staticpod/controller/guard" "github.com/openshift/library-go/pkg/operator/staticpod/controller/installer" "github.com/openshift/library-go/pkg/operator/staticpod/controller/installerstate" "github.com/openshift/library-go/pkg/operator/staticpod/controller/node" @@ -62,6 +63,12 @@ type staticPodOperatorControllerBuilder struct { pruneCommand []string // TODO de-dupe this. I think it's actually a directory name staticPodPrefix string + + // guard infomation + operatorName string + operatorNamespace string + readyzPort string + guardCreateConditionalFunc func() (bool, error) } func NewBuilder( @@ -91,6 +98,7 @@ type Builder interface { // the installer pod is created for a revision. WithCustomInstaller(command []string, installerPodMutationFunc installer.InstallerPodMutationFunc) Builder WithPruning(command []string, staticPodPrefix string) Builder + WithPodDisruptionBudgetGuard(operatorNamespace, operatorName, readyzPort string, createConditionalFunc func() (bool, error)) Builder ToControllers() (manager.ControllerManager, error) } @@ -153,6 +161,14 @@ func (b *staticPodOperatorControllerBuilder) WithPruning(command []string, stati return b } +func (b *staticPodOperatorControllerBuilder) WithPodDisruptionBudgetGuard(operatorNamespace, operatorName, readyzPort string, createConditionalFunc func() (bool, error)) Builder { + b.operatorNamespace = operatorNamespace + b.operatorName = operatorName + b.readyzPort = readyzPort + b.guardCreateConditionalFunc = createConditionalFunc + return b +} + func (b *staticPodOperatorControllerBuilder) ToControllers() (manager.ControllerManager, error) { manager := manager.NewControllerManager() @@ -169,6 +185,7 @@ func (b *staticPodOperatorControllerBuilder) ToControllers() (manager.Controller secretClient := v1helpers.CachedSecretGetter(b.kubeClient.CoreV1(), b.kubeInformers) podClient := b.kubeClient.CoreV1() eventsClient := b.kubeClient.CoreV1() + pdbClient := b.kubeClient.PolicyV1() operandInformers := b.kubeInformers.InformersFor(b.operandNamespace) clusterInformers := b.kubeInformers.InformersFor("") @@ -299,5 +316,21 @@ func (b *staticPodOperatorControllerBuilder) ToControllers() (manager.Controller manager.WithController(unsupportedconfigoverridescontroller.NewUnsupportedConfigOverridesController(b.staticPodOperatorClient, eventRecorder), 1) manager.WithController(loglevel.NewClusterOperatorLoggingController(b.staticPodOperatorClient, eventRecorder), 1) + if len(b.operatorNamespace) > 0 && len(b.operatorName) > 0 && len(b.readyzPort) > 0 { + manager.WithController(guard.NewGuardController( + b.operandNamespace, + b.staticPodName, + b.operatorName, + b.readyzPort, + operandInformers, + clusterInformers, + b.staticPodOperatorClient, + podClient, + pdbClient, + eventRecorder, + b.guardCreateConditionalFunc, + ), 1) + } + return manager, errors.NewAggregate(errs) }