🌱 chore(deps): upgrade k8s to 1.35 and Go dependencies (edge)

[camilamacedo86]

Description

Reviewer Checklist

• API Go Documentation
• Tests: Unit Tests (and E2E Tests, if appropriate)
• Comprehensive Commit Messages
• Links to related GitHub Issue(s)

[netlify]

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	efbe1f6
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/698b804fb743d10008cb22b5
😎 Deploy Preview	https://deploy-preview-2486--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[Copilot]

Copilot wasn't able to review any files in this pull request.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.44%. Comparing base (28102fa) to head (efbe1f6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
test/utils.go	0.00%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2486      +/-   ##
==========================================
- Coverage   73.74%   69.44%   -4.30%     
==========================================
  Files         102      102              
  Lines        8496     8504       +8     
==========================================
- Hits         6265     5906     -359     
- Misses       1747     2129     +382     
+ Partials      484      469      -15     

Flag	Coverage Δ
e2e	45.59% <100.00%> (-0.65%)	⬇️
experimental-e2e	12.72% <100.00%> (-41.32%)	⬇️
unit	57.89% <27.77%> (-0.09%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[camilamacedo86]

TODO: Lets looking on that in a follow up shows very complex to go in this PR

[perdasilva]

is it likely that the final attempt works? could we be calling out a false negative with the previous log line?

[perdasilva]

maybe we should just return:

fmt.Errorf("StopWithRetry: timeout reached before successful teardown; last error: %v", lastErr)

?

[camilamacedo86]

Yes, it works as it is. Within controller-runtime v0.23. has some timing changes in the test environment teardown process. Without using Eventually, testEnv.Stop() can fail intermittently with errors related to graceful shutdown timing.

So this code does the same of:

var _ = AfterSuite(func() {
    By("tearing down the test environment")
    cancel()
    Eventually(func() error {
        return testEnv.Stop()
    }, time.Minute, time.Second).Should(Succeed())
})

[perdasilva]

My question is whether we can be in a situation where

StopWithRetry: timeout reached before successful teardown; last error: ... is printed out but actually it stopped successfully in the final attempt (and whether we care)

[perdasilva]

maybe just do

err := env.Stop() // Final attempt
if err != nil {
   log ...
}
return err

then you also don't need to keep the lastErr

[camilamacedo86]

Done.

[pedjak]

/lgtm

[openshift-ci]

New changes are detected. LGTM label has been removed.

[camilamacedo86]

I asked for Claude evaluate those changes
Following bellow.

The Problem (WHY the change was needed)

Old Implementation (main branch)

t.Log("Waiting for acquired leader election")
leaderCtx, leaderCancel := context.WithTimeout(ctx, 3*time.Minute)
defer leaderCancel()
leaderSubstrings := []string{"successfully acquired lease"}
leaderElected, err := watchPodLogsForSubstring(leaderCtx, &managerPod, leaderSubstrings...)
require.NoError(t, err)
require.True(t, leaderElected)

The watchPodLogsForSubstring function used:

podLogOpts := corev1.PodLogOptions{
    Follow:    true,  // <-- THIS IS THE PROBLEM
    Container: container,
}

The Race Condition

1. controller-runtime v0.23.1+ has FASTER leader election startup
2. The test starts AFTER the pod is created
3. By the time the test calls watchPodLogsForSubstring, leader election has already completed
4. Follow: true only streams NEW logs, missing messages already emitted
5. The test waits forever for a message that already passed

Timeline Visualization

controller-runtime v0.22.x (SLOW startup):
├─ Pod starts
├─ Test starts watching logs (Follow=true)
├─ Leader election happens  ← Message logged and caught by test ✅
└─ Test passes

controller-runtime v0.23.1+ (FAST startup):
├─ Pod starts
├─ Leader election happens  ← Message logged
├─ Test starts watching logs (Follow=true)  ← Too late! Message missed ❌
└─ Test times out waiting for message

New Implementation (deps-up branch)

Approach 1: Direct Lease Checking (for leader election verification)

t.Logf("Waiting for acquired leader election by checking lease")
// Instead of watching logs (which has timing issues in controller-runtime v0.23.1+),
// directly check the lease object to confirm leader election occurred
require.EventuallyWithT(t, func(ct *assert.CollectT) {
    var lease coordinationv1.Lease
    err := c.Get(ctx, types.NamespacedName{Name: "catalogd-operator-lock", Namespace: "olmv1-system"}, &lease)
    require.NoError(ct, err)
    require.NotNil(ct, lease.Spec.HolderIdentity, "lease should have a holder")
    require.NotEmpty(ct, *lease.Spec.HolderIdentity, "lease holder identity should not be empty")
}, 3*time.Minute, time.Second)

Benefits:

• ✅ Timing-independent - checks the actual state, not log messages
• ✅ More reliable - verifies the desired outcome (lease acquired)
• ✅ No race conditions - lease object persists
• ✅ Tests the right thing - we care about the lease, not the log message

Approach 2: Polling with TailLines (for other log messages)

For messages that MUST be verified from logs (e.g., "reconcile ending"), the implementation was changed to use polling:

func watchPodLogsForSubstring(ctx context.Context, pod *corev1.Pod, substrings ...string) (bool, error) {
    // Use a polling approach to periodically check pod logs for the substrings
    // This handles controller-runtime v0.23.1+ where leader election happens faster
    // and the message may be emitted before we start watching with Follow
    ticker := time.NewTicker(2 * time.Second)
    defer ticker.Stop()

    for {
        select {
        case <-ctx.Done():
            return false, ctx.Err()
        case <-ticker.C:
            // Get recent logs without Follow to check if message exists
            logOpts := corev1.PodLogOptions{
                Container: container,
                TailLines: ptr.To(int64(1000)),  // <-- Check EXISTING logs
            }
            // ... check logs for substring ...
        }
    }
}

Benefits:

• ✅ Checks existing logs, not just new ones
• ✅ Handles messages emitted before watching starts
• ✅ Still timing-independent

Behavior Verification

Old Behavior

• ❌ Waited for "successfully acquired lease" log message
• ❌ Could miss fast leader elections
• ❌ Relied on timing

New Behavior

• ✅ Checks that lease.Spec.HolderIdentity is set
• ✅ Verifies leader election occurred (same goal, better method)
• ✅ Independent of timing

Are they equivalent?

YES - Both verify that leader election completed successfully:

• Old: "If log says 'successfully acquired lease', then leader election worked"
• New: "If lease has a HolderIdentity, then leader election worked"

The new approach is actually MORE accurate because:

1. It checks the actual state object, not a log message
2. It can't be affected by log rotation, buffering, or truncation
3. It's the single source of truth for leader election status

[Copilot]

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.