Skip to content

Commit

Permalink
Changed the signature of ensureInstallerPod to also include requeue; …
Browse files Browse the repository at this point in the history 
…Updated the tests

Signed-off-by: jubittajohn <jujohn@redhat.com>
  • Loading branch information
@jubittajohn
jubittajohn committed Jun 26, 2024
1 parent dd98d73 commit 713d859
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 27 deletions.
30 changes: 21 additions & 9 deletions pkg/operator/staticpod/controller/installer/installer_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ type InstallerController struct {
installerBackOff func(count int) time.Duration
fallbackBackOff func(count int) time.Duration

// shouldRevisionInstall is a callback function that determines whether a new revision should be installed
shouldRevisionInstall func() (bool, error)
}

Expand Down Expand Up @@ -485,14 +486,19 @@ func (c *InstallerController) manageInstallationPods(ctx context.Context, operat
}
}

if err := c.ensureInstallerPod(ctx, operatorSpec, currNodeState); err != nil {
requeue, err := c.ensureInstallerPod(ctx, operatorSpec, currNodeState)
if err != nil {
c.eventRecorder.Warningf("InstallerPodFailed", "Failed to create installer pod for revision %d count %d on node %q: %v",
currNodeState.TargetRevision, currNodeState.LastFailedCount, currNodeState.NodeName, err)
// if a newer revision is pending, continue, so we retry later with the latest available revision
if !(operatorStatus.LatestAvailableRevision > currNodeState.TargetRevision) {
return true, 0, err
}
}
if requeue {
klog.V(4).Infof("Requeuing the creation of installer pod for revision %d on node %q", currNodeState.TargetRevision, currNodeState.NodeName)
return true, 0, nil
}
}

newCurrNodeState, _, reason, err := c.newNodeStateForInstallInProgress(ctx, currNodeState, operatorStatus.LatestAvailableRevision)
Expand Down Expand Up @@ -858,12 +864,16 @@ func getInstallerPodName(ns *operatorv1.NodeStatus) string {
}

// ensureInstallerPod creates the installer pod with the secrets required to if it does not exist already
func (c *InstallerController) ensureInstallerPod(ctx context.Context, operatorSpec *operatorv1.StaticPodOperatorSpec, ns *operatorv1.NodeStatus) error {
// returns whether or not requeue and if an error happened while creating installer pod
func (c *InstallerController) ensureInstallerPod(ctx context.Context, operatorSpec *operatorv1.StaticPodOperatorSpec, ns *operatorv1.NodeStatus) (bool, error) {
// checks if new revision should be rolled out
if c.shouldRevisionInstall != nil {
shouldInstall, err := c.shouldRevisionInstall()
if err != nil {
return true, err
}
if !shouldInstall {
return err
return true, nil
}
}
pod := resourceread.ReadPodV1OrDie(podTemplate)
Expand All @@ -876,19 +886,19 @@ func (c *InstallerController) ensureInstallerPod(ctx context.Context, operatorSp

ownerRefs, err := c.ownerRefsFn(ctx, ns.TargetRevision)
if err != nil {
return fmt.Errorf("unable to set installer pod ownerrefs: %+v", err)
return true, fmt.Errorf("unable to set installer pod ownerrefs: %+v", err)
}
pod.OwnerReferences = ownerRefs

if c.configMaps[0].Optional {
return fmt.Errorf("pod configmap %s is required, cannot be optional", c.configMaps[0].Name)
return true, fmt.Errorf("pod configmap %s is required, cannot be optional", c.configMaps[0].Name)
}

// if the startup monitor is enabled we need to acquire an exclusive lock
// to coordinate the work between the installer and the monitor
withStartupMonitorSupport, err := c.startupMonitorEnabled()
if err != nil {
return fmt.Errorf("unable to determine if the startup monitor should be enabled: %v", err)
return true, fmt.Errorf("unable to determine if the startup monitor should be enabled: %v", err)
}

args := []string{
Expand Down Expand Up @@ -940,12 +950,14 @@ func (c *InstallerController) ensureInstallerPod(ctx context.Context, operatorSp
// Some owners need to change aspects of the pod. Things like arguments for instance
for _, fn := range c.installerPodMutationFns {
if err := fn(pod, ns.NodeName, operatorSpec, ns.TargetRevision); err != nil {
return err
return true, err
}
}

_, _, err = resourceapply.ApplyPod(ctx, c.podsGetter, c.eventRecorder, pod)
return err
if _, _, err = resourceapply.ApplyPod(ctx, c.podsGetter, c.eventRecorder, pod); err != nil {
return true, err
}
return false, nil
}

func (c *InstallerController) setOwnerRefs(ctx context.Context, revision int32) ([]metav1.OwnerReference, error) {
Expand Down
50 changes: 32 additions & 18 deletions pkg/operator/staticpod/controller/installer/installer_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1268,7 +1268,7 @@ func TestEnsureInstallerPod(t *testing.T) {
c.ownerRefsFn = func(ctx context.Context, revision int32) ([]metav1.OwnerReference, error) {
return []metav1.OwnerReference{}, nil
}
err := c.ensureInstallerPod(context.TODO(), &operatorv1.StaticPodOperatorSpec{}, &operatorv1.NodeStatus{
_, err := c.ensureInstallerPod(context.TODO(), &operatorv1.StaticPodOperatorSpec{}, &operatorv1.NodeStatus{
NodeName: "test-node-1",
TargetRevision: 1,
})
Expand Down Expand Up @@ -2086,58 +2086,64 @@ func TestInstallerController_manageInstallationPods(t *testing.T) {
wantErr bool
}{
{
name: "if the revision shouldn't install and a new revision is to be rolled out, requeue the new installer pod",
name: "if the revision shouldn't install and a new revision is to be rolled out, requeue the creation of installer pod",
fields: fields{
targetNamespace: "test-namespace",
staticPodName: "test-pod",
command: []string{},
kubeClient: fake.NewSimpleClientset(),
eventRecorder: eventstesting.NewTestingEventRecorder(t),
shouldRevisionInstall: func() (bool, error) {
return false, fmt.Errorf("revision shouldn't be installed")
return false, nil
},
},
args: args{
operatorSpec: &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{},
},
originalOperatorStatus: &operatorv1.StaticPodOperatorStatus{
LatestAvailableRevision: 1,
LatestAvailableRevision: 4,
NodeStatuses: []operatorv1.NodeStatus{
{
NodeName: "test-node-0",
CurrentRevision: 4,
},
{
NodeName: "test-node-1",
CurrentRevision: 1,
TargetRevision: 2,
LastFailedCount: 1,
CurrentRevision: 3,
TargetRevision: 4,
},
},
},
},
want: true,
wantErr: true,
wantErr: false,
},
{
name: "if the revision shouldn't install and no new revision is to be rolled out, then no requeue",
name: "if the revision shouldn't install and no new revision is to be rolled out, then no requeue of the installer pod",
fields: fields{
targetNamespace: "test-namespace",
staticPodName: "test-pod",
command: []string{},
kubeClient: fake.NewSimpleClientset(),
shouldRevisionInstall: func() (bool, error) {
return false, fmt.Errorf("revision shouldn't be installed")
return false, nil
},
},
args: args{
operatorSpec: &operatorv1.StaticPodOperatorSpec{
OperatorSpec: operatorv1.OperatorSpec{},
},
originalOperatorStatus: &operatorv1.StaticPodOperatorStatus{
LatestAvailableRevision: 1,
LatestAvailableRevision: 3,
NodeStatuses: []operatorv1.NodeStatus{
{
NodeName: "test-node-0",
CurrentRevision: 3,
},
{
NodeName: "test-node-1",
CurrentRevision: 1,
TargetRevision: 1,
CurrentRevision: 3,
},
},
},
Expand All @@ -2146,7 +2152,7 @@ func TestInstallerController_manageInstallationPods(t *testing.T) {
wantErr: false,
},
{
name: "if the revision is safe to install and a new revision is to be rolled out, no requeue",
name: "if the revision should install and a new revision is to be rolled out, no requeue of the installer pod",
fields: fields{
targetNamespace: "test-namespace",
staticPodName: "test-pod",
Expand All @@ -2166,12 +2172,20 @@ func TestInstallerController_manageInstallationPods(t *testing.T) {
OperatorSpec: operatorv1.OperatorSpec{},
},
originalOperatorStatus: &operatorv1.StaticPodOperatorStatus{
LatestAvailableRevision: 1,
LatestAvailableRevision: 4,
NodeStatuses: []operatorv1.NodeStatus{
{
NodeName: "test-node-1",
CurrentRevision: 1,
TargetRevision: 2,
NodeName: "test-node-0",
CurrentRevision: 3,
TargetRevision: 4,
},
{
NodeName: "test-node-0",
CurrentRevision: 3,
},
{
NodeName: "test-node-0",
CurrentRevision: 3,
},
},
},
Expand Down

0 comments on commit 713d859

Please sign in to comment.