Skip to content

OCPBUGS-65626: add service account to guard pod #2076

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

Open
ehearne-redhat wants to merge 1 commit into
base: master
Choose a base branch
from
Open

OCPBUGS-65626: add service account to guard pod #2076

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
65 changes: 65 additions & 0 deletions pkg/operator/staticpod/controller/guard/guard_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ type GuardController struct {
podGetter corev1client.PodsGetter
pdbGetter policyclientv1.PodDisruptionBudgetsGetter
pdbLister policylisterv1.PodDisruptionBudgetLister
saLister corelisterv1.ServiceAccountLister
saGetter corev1client.ServiceAccountsGetter

// installerPodImageFn returns the image name for the installer pod
installerPodImageFn func() string
Expand All @@ -70,6 +72,7 @@ func NewGuardController(
operatorClient operatorv1helpers.StaticPodOperatorClient,
podGetter corev1client.PodsGetter,
pdbGetter policyclientv1.PodDisruptionBudgetsGetter,
saGetter corev1client.ServiceAccountsGetter,
eventRecorder events.Recorder,
createConditionalFunc func() (bool, bool, error),
extraNodeSelector labels.Selector,
Expand Down Expand Up @@ -102,6 +105,8 @@ func NewGuardController(
podGetter: podGetter,
pdbGetter: pdbGetter,
pdbLister: kubeInformersForTargetNamespace.Policy().V1().PodDisruptionBudgets().Lister(),
saGetter: saGetter,
saLister: kubeInformersForTargetNamespace.Core().V1().ServiceAccounts().Lister(),
installerPodImageFn: getInstallerPodImageFromEnv,
createConditionalFunc: createConditionalFunc,
extraNodeSelector: extraNodeSelector,
Expand Down Expand Up @@ -195,6 +200,29 @@ func (c *GuardController) sync(ctx context.Context, syncCtx factory.SyncContext)
return nil
}

// Do a check for zombie service accounts if a guard pod gets manually
// deleted by the user.
pods, _ := c.podLister.Pods(c.targetNamespace).List(labels.SelectorFromSet(labels.Set{"app": "guard"}))

podMap := map[string]bool{}

for _, pod := range pods {
podMap[pod.Spec.ServiceAccountName] = true
}

serviceAccounts, _ := c.saLister.ServiceAccounts(c.targetNamespace).List(labels.SelectorFromSet(labels.Set{"app": "guard"}))

for _, sa := range serviceAccounts {
// If service account exists but pod map does not have a key for it,
// it is a zombie service account, and must be deleted.
if !podMap[sa.Name] {
err = c.saGetter.ServiceAccounts(c.targetNamespace).Delete(ctx, sa.Name, metav1.DeleteOptions{})
if err != nil {
klog.Errorf("Error while deleting service account %s : %v", sa.Name, err)
}
}
}

errs := []error{}
if !shouldCreate {
pdb := resourceread.ReadPodDisruptionBudgetV1OrDie(pdbTemplate)
Expand Down Expand Up @@ -232,6 +260,20 @@ func (c *GuardController) sync(ctx context.Context, syncCtx factory.SyncContext)
}
}
}

// delete service accounts associated with guard pod(s)
serviceAccounts, err := c.saLister.ServiceAccounts(c.targetNamespace).List(labels.SelectorFromSet(labels.Set{"app": "guard"}))
if err != nil {
errs = append(errs, err)
} else {
for _, sa := range serviceAccounts {
_, _, err = resourceapply.DeleteServiceAccount(ctx, c.saGetter, syncCtx.Recorder(), sa)
if err != nil {
klog.Errorf("Unable to delete Service Account: %v", err)
errs = append(errs, err)
}
}
}
} else {
nodes, err := c.nodeLister.List(c.masterNodesSelector)
if err != nil {
Expand Down Expand Up @@ -320,6 +362,24 @@ func (c *GuardController) sync(ctx context.Context, syncCtx factory.SyncContext)

klog.V(5).Infof("Rendering guard pod for operand %v on node %v", operands[node.Name].Name, node.Name)

// Create Service Account
serviceAccount := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: getGuardPodName(c.podResourcePrefix, node.Name),
Namespace: c.targetNamespace,
},
}
// attach label so we can recognise for deletion later
serviceAccount.Labels = map[string]string{
"app": "guard",
}
_, _, err = resourceapply.ApplyServiceAccount(ctx, c.saGetter, syncCtx.Recorder(), serviceAccount)

if err != nil {
klog.Errorf("Unable to create service account %v for Guard Pod: %v", serviceAccount.Name, err)
errs = append(errs, fmt.Errorf("Unable to create service account %v for Guard Pod: %v", serviceAccount.Name, err))
}

pod := resourceread.ReadPodV1OrDie(podTemplate)

pod.ObjectMeta.Name = getGuardPodName(c.podResourcePrefix, node.Name)
Expand All @@ -328,6 +388,7 @@ func (c *GuardController) sync(ctx context.Context, syncCtx factory.SyncContext)
pod.Spec.NodeName = node.Name
pod.Spec.Containers[0].Image = c.installerPodImageFn()
pod.Spec.Containers[0].ReadinessProbe.HTTPGet.Host = operands[node.Name].Status.PodIP
pod.Spec.ServiceAccountName = serviceAccount.Name
// The readyz port as string type is expected to be convertible into int!!!
readyzPort, err := strconv.Atoi(c.readyzPort)
if err != nil {
Expand Down Expand Up @@ -364,6 +425,10 @@ func (c *GuardController) sync(ctx context.Context, syncCtx factory.SyncContext)
klog.V(5).Infof("Pod phase is neither pending nor running, deleting %v so the guard can be re-created", pod.Name)
delete = true
}
if actual.Spec.ServiceAccountName != pod.Spec.ServiceAccountName {
klog.V(5).Infof("Service Account 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 {
Expand Down
59 changes: 58 additions & 1 deletion pkg/operator/staticpod/controller/guard/guard_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ func TestRenderGuardPod(t *testing.T) {
node *corev1.Node
guardExists bool
guardPod *corev1.Pod
guardServiceAccount *corev1.ServiceAccount
createConditionalFunc func() (bool, bool, error)
withArbiter bool
}{
Expand Down Expand Up @@ -427,6 +428,13 @@ func TestRenderGuardPod(t *testing.T) {
PodIP: "1.1.1.1",
},
},
guardServiceAccount: &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: getGuardPodName("operand", "master1"),
Namespace: "test",
Labels: map[string]string{"app": "guard"},
},
},
node: fakeMasterNode("master1"),
},
{
Expand Down Expand Up @@ -466,6 +474,13 @@ func TestRenderGuardPod(t *testing.T) {
PodIP: "1.1.1.1",
},
},
guardServiceAccount: &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: getGuardPodName("operand", "arbiter"),
Namespace: "test",
Labels: map[string]string{"app": "guard"},
},
},
//createConditionalFunc: func() (bool, bool, error) { return true, true, nil },
node: fakeArbiterNode("arbiter"),
withArbiter: true,
Expand Down Expand Up @@ -522,6 +537,13 @@ func TestRenderGuardPod(t *testing.T) {
Phase: corev1.PodSucceeded,
},
},
guardServiceAccount: &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: getGuardPodName("operand", "master1"),
Namespace: "test",
Labels: map[string]string{"app": "guard"},
},
},
node: fakeMasterNode("master1"),
guardExists: true,
},
Expand Down Expand Up @@ -574,6 +596,13 @@ func TestRenderGuardPod(t *testing.T) {
Phase: corev1.PodSucceeded,
},
},
guardServiceAccount: &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: getGuardPodName("operand", "arbiter"),
Namespace: "test",
Labels: map[string]string{"app": "guard"},
},
},
createConditionalFunc: func() (bool, bool, error) { return true, true, nil },
node: fakeArbiterNode("arbiter"),
guardExists: true,
Expand Down Expand Up @@ -622,6 +651,9 @@ func TestRenderGuardPod(t *testing.T) {
if test.guardPod != nil {
kubeClient.Tracker().Add(test.guardPod)
}
if test.guardServiceAccount != nil {
kubeClient.Tracker().Add(test.guardServiceAccount)
}
kubeInformers := informers.NewSharedInformerFactoryWithOptions(kubeClient, 1*time.Minute)
eventRecorder := events.NewRecorder(kubeClient.CoreV1().Events("test"), "test-operator", &corev1.ObjectReference{}, clocktesting.NewFakePassiveClock(time.Now()))

Expand Down Expand Up @@ -651,6 +683,8 @@ func TestRenderGuardPod(t *testing.T) {
podGetter: kubeClient.CoreV1(),
pdbGetter: kubeClient.PolicyV1(),
pdbLister: kubeInformers.Policy().V1().PodDisruptionBudgets().Lister(),
saGetter: kubeClient.CoreV1(),
saLister: kubeInformers.Core().V1().ServiceAccounts().Lister(),
installerPodImageFn: getInstallerPodImageFromEnv,
createConditionalFunc: createConditionalFunc,
}
Expand Down Expand Up @@ -696,12 +730,26 @@ func TestRenderGuardPod(t *testing.T) {
if p.Status.Phase != "" {
t.Errorf("%s: unexpected pod status: %v, expected no status set", test.name, p.Status.Phase)
}
// check to ensure service account is created for pod.
expectedSAName := getGuardPodName("operand", test.node.Name)
if p.Spec.ServiceAccountName != expectedSAName {
t.Errorf("%s: expected ServiceAccountName %q, got %q", test.name, expectedSAName, p.Spec.ServiceAccountName)
}
_, saErr := kubeClient.CoreV1().ServiceAccounts("test").Get(ctx, p.Spec.ServiceAccountName, metav1.GetOptions{})
if saErr != nil {
t.Errorf("%s: expected ServiceAccount %q to exist, got error instead: %q", test.name, p.Spec.ServiceAccountName, saErr)
}
}
} 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)
}
// verify service account is not found (i.e. deleted)
_, err = kubeClient.CoreV1().ServiceAccounts("test").Get(ctx, getGuardPodName("operand", "master1"), metav1.GetOptions{})
if !apierrors.IsNotFound(err) {
t.Errorf("%s: expected 'service account \"%v\" not found' error, but got %q instead", test.name, getGuardPodName("operand", "master1"), err)
}
}
}
})
Expand Down Expand Up @@ -759,13 +807,20 @@ func TestRenderGuardPodPortChanged(t *testing.T) {
PodIP: "1.1.1.1",
},
}
guardServiceAccount := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: getGuardPodName("operand", "master1"),
Namespace: "test",
Labels: map[string]string{"app": "guard"},
},
}

indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
if err := indexer.Add(infraObject); err != nil {
t.Fatal(err.Error())
}

kubeClient := fake.NewSimpleClientset(fakeMasterNode("master1"), operandPod, guardPod)
kubeClient := fake.NewSimpleClientset(fakeMasterNode("master1"), operandPod, guardPod, guardServiceAccount)
kubeInformers := informers.NewSharedInformerFactoryWithOptions(kubeClient, 1*time.Minute)
eventRecorder := events.NewRecorder(kubeClient.CoreV1().Events("test"), "test-operator", &corev1.ObjectReference{}, clocktesting.NewFakePassiveClock(time.Now()))

Expand All @@ -791,6 +846,8 @@ func TestRenderGuardPodPortChanged(t *testing.T) {
podGetter: kubeClient.CoreV1(),
pdbGetter: kubeClient.PolicyV1(),
pdbLister: kubeInformers.Policy().V1().PodDisruptionBudgets().Lister(),
saGetter: kubeClient.CoreV1(),
saLister: kubeInformers.Core().V1().ServiceAccounts().Lister(),
installerPodImageFn: getInstallerPodImageFromEnv,
createConditionalFunc: staticcontrollercommon.NewIsSingleNodePlatformFn(informer),
}
Expand Down
1 change: 1 addition & 0 deletions pkg/operator/staticpod/controller/guard/manifests/guard-pod.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ metadata:
app: guard
ownerReferences: # Value set by operator
spec:
serviceAccountName: # Value set by operator
affinity: # Value set by operator
priorityClassName: "system-cluster-critical"
terminationGracePeriodSeconds: 3
Expand Down
2 changes: 2 additions & 0 deletions pkg/operator/staticpod/controllers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ func (b *staticPodOperatorControllerBuilder) ToControllers() (manager.Controller
podClient := b.kubeClient.CoreV1()
eventsClient := b.kubeClient.CoreV1()
pdbClient := b.kubeClient.PolicyV1()
saClient := b.kubeClient.CoreV1()
operandInformers := b.kubeNamespaceInformers.InformersFor(b.operandNamespace)
clusterInformers := b.kubeClusterInformers
infraInformers := b.configInformers.Config().V1().Infrastructures()
Expand Down Expand Up @@ -398,6 +399,7 @@ func (b *staticPodOperatorControllerBuilder) ToControllers() (manager.Controller
b.staticPodOperatorClient,
podClient,
pdbClient,
saClient,
eventRecorder,
b.guardCreateConditionalFunc,
b.extraNodeSelector,
Expand Down