Skip to content

Commit e920064

Browse files
committed
oidc: only wait for KAS rollout if we actually modified the Authentication resource
Signed-off-by: Bryce Palmer <[email protected]>
1 parent 856479f commit e920064

File tree

1 file changed

+20
-8
lines changed
  • test/extended/authentication

1 file changed

+20
-8
lines changed

test/extended/authentication/oidc.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
exutil "github.com/openshift/origin/test/extended/util"
1616
authnv1 "k8s.io/api/authentication/v1"
1717
corev1 "k8s.io/api/core/v1"
18+
"k8s.io/apimachinery/pkg/api/equality"
1819
apierrors "k8s.io/apimachinery/pkg/api/errors"
1920
"k8s.io/apimachinery/pkg/util/errors"
2021

@@ -184,10 +185,12 @@ var _ = g.Describe("[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow]
184185
gomega.Expect(err).NotTo(o.HaveOccurred(), "should be able to create a SelfSubjectReview")
185186
}).WithTimeout(5 * time.Minute).WithPolling(10 * time.Second).Should(o.Succeed())
186187

187-
err := resetAuthentication(ctx, oc, originalAuth)
188+
err, modified := resetAuthentication(ctx, oc, originalAuth)
188189
o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error reverting authentication to original state")
189190

190-
waitForRollout(ctx, oc)
191+
if modified {
192+
waitForRollout(ctx, oc)
193+
}
191194
})
192195

193196
g.It("should rollout configuration on the kube-apiserver successfully", func() {
@@ -364,10 +367,13 @@ var _ = g.Describe("[sig-auth][Suite:openshift/auth/external-oidc][Serial][Slow]
364367
err := removeResources(ctx, cleanups...)
365368
o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error cleaning up keycloak resources")
366369

367-
err = resetAuthentication(ctx, oc, originalAuth)
370+
err, modified := resetAuthentication(ctx, oc, originalAuth)
368371
o.Expect(err).NotTo(o.HaveOccurred(), "should not encounter an error reverting authentication to original state")
369372

370-
waitForRollout(ctx, oc)
373+
// Only if we modified the Authentication resource during the reset should we wait for a rollout
374+
if modified {
375+
waitForRollout(ctx, oc)
376+
}
371377
})
372378
})
373379

@@ -478,11 +484,12 @@ func admittedURLForRoute(ctx context.Context, client *exutil.CLI, routeName, nam
478484
return fmt.Sprintf("https://%s", admittedURL), err
479485
}
480486

481-
func resetAuthentication(ctx context.Context, client *exutil.CLI, original *configv1.Authentication) error {
487+
func resetAuthentication(ctx context.Context, client *exutil.CLI, original *configv1.Authentication) (error, bool) {
482488
if original == nil {
483-
return nil
489+
return nil, false
484490
}
485491

492+
modified := false
486493
timeoutCtx, cancel := context.WithDeadline(ctx, time.Now().Add(5*time.Minute))
487494
defer cancel()
488495
cli := client.AdminConfigClient().ConfigV1().Authentications()
@@ -492,19 +499,24 @@ func resetAuthentication(ctx context.Context, client *exutil.CLI, original *conf
492499
return false, fmt.Errorf("getting the current authentications.config.openshift.io/cluster: %w", err)
493500
}
494501

502+
if equality.Semantic.DeepEqual(current.Spec, original.Spec) {
503+
return true, nil
504+
}
505+
495506
current.Spec = original.Spec
507+
modified = true
496508

497509
_, err = cli.Update(ctx, current, metav1.UpdateOptions{})
498510
if err != nil {
499511
// Only log the error so we continue to retry until the context has timed out
500-
g.GinkgoLogr.Error(err, "updating authentication resource")
512+
fmt.Println("updating authentication resource:", err)
501513
return false, nil
502514
}
503515

504516
return true, nil
505517
})
506518

507-
return err
519+
return err, modified
508520
}
509521

510522
func waitForRollout(ctx context.Context, client *exutil.CLI) {

0 commit comments

Comments
 (0)