Fix kube discovery service `poll_interval` value not set correctly by kshi36 · Pull Request #60982 · gravitational/teleport

kshi36 · 2025-11-04T00:02:41Z

Manual Tests

Test: Specify a poll interval not equal to the default value of 5 minutes

Verify that before the change, discovery service polls every 5 minutes regardless of poll_interval field
Verify that after the change, discovery service polls based on value of poll_interval field

Examples:

config file

app_service:
      enabled: true
      resources:
      - labels:
          teleport.dev/kubernetes-cluster: main-cluster
          teleport.dev/origin: discovery-kubernetes
    auth_service:
      enabled: false
    db_service:
      enabled: false
    discovery_service:
      discovery_group: main-cluster
      enabled: true
      kubernetes:
      - labels:
          '*': '*'
        namespaces:
        - '*'
        types:
        - app
      poll_interval: 1m
    jamf_service:
      enabled: false
    kubernetes_service:
      enabled: true
      kube_cluster_name: main-cluster
    proxy_service:
      enabled: false
    ssh_service:
      enabled: false
    teleport:
      join_params:
        method: token
        token_name: /etc/teleport-secrets/auth-token
      log:
        format:
          extra_fields:
          - timestamp
          - level
          - component
          - caller
          output: text
        output: stderr
        severity: INFO
      proxy_server: kevin.dev:443
    version: v3

result

2025-11-14T17:09:18.103Z INFO [DISCOVERY] New resource matches, creating pid:11.1 kind:app kind:app name:dummy-service-no-pods-5-default-main-cluster services/reconciler.go:264
2025-11-14T17:09:18.925Z INFO [APP:SERVI] New resource matches, creating kind:app kind:app name:dummy-service-no-pods-5-default-main-cluster services/reconciler.go:264
2025-11-14T17:10:18.112Z INFO [DISCOVERY] Resource was removed, deleting pid:11.1 kind:app kind:app name:dummy-service-no-pods-5-default-main-cluster services/reconciler.go:219
2025-11-14T17:10:18.925Z INFO [APP:SERVI] Resource was removed, deleting kind:app kind:app name:dummy-service-no-pods-5-default-main-cluster services/reconciler.go:219

changelog: Fixed bug where Kubernetes App Discovery poll_interval is not set correctly

rosstimothy · 2025-11-12T17:33:19Z

@kshi36 could you update your PR description with the manual testing that you did to validate this change? See #59850 for an example.

lib/srv/discovery/discovery_test.go

kshi36 · 2025-11-17T20:00:53Z

lib/srv/discovery/discovery_eks_test.go

+func (f *mockAuthServer) CreateApp(ctx context.Context, _ types.Application) error {
+	return nil
+}
+
+func (m *mockAuthServer) GetApps(ctx context.Context) ([]types.Application, error) {
+	return nil, nil
+}
+
+func (m *mockAuthServer) ListApps(ctx context.Context, limit int, startKey string) ([]types.Application, string, error) {
+	return nil, "", nil
+}
+
+func (m *mockAuthServer) RangeApps(ctx context.Context, start, end string) iter.Seq2[types.Application, error] {
+	return func(yield func(types.Application, error) bool) {}
+}
+


Currently, I have extended the mockAuthServer implementation here for use in my test in discovery_test.go, since it contains similar logic for my usage with synctest. I felt this was the best way to write the test without interfering with existing test logic in discovery_test.go. I specifically needed the functionality of a mock AccessPoint to eliminate non-durably blocking gRPC calls that impact synctest.

In a future PR, I want to migrate everything in discovery_eks_test.go back to discovery_test.go, and modify existing tests to utilize synctest (and use the same mockAuthServer implementation), but this would be extra scope not intended for this PR.

rosstimothy

Thanks for taking the time to convert the new test to use synctest @kshi36! The changes look good and I've validated them locally as well, just a few further comments I'd like to see addressed before approving.

lib/srv/discovery/discovery_test.go

lib/srv/discovery/discovery_eks_test.go

lib/srv/discovery/discovery_test.go

rosstimothy

Nice work @kshi36!

kshi36 · 2025-11-17T22:13:57Z

It seems that converting the deprecated fake.NewSimpleClientset() to fake.NewClientset() introduced flakiness, unless there's something else I'm missing. I will consider reverting previous tests to the old call

rosstimothy · 2025-11-17T22:17:05Z

It seems that converting the deprecated fake.NewSimpleClientset() to fake.NewClientset() introduced flakiness, unless there's something else I'm missing. I will consider reverting previous tests to the old call

Using NewClientset is not the source of the flakiness, the number of locations that have been modified to use it is. The Flaky Test Detector runs all modified tests N times and expects them to complete in M seconds. You've modified so many tests that they no cannot complete fast enough.

We can either change these one test at a time, or we can bypass the flaky test detector on this PR as is.

kshi36 · 2025-11-17T22:25:22Z

We can either change these one test at a time, or we can bypass the flaky test detector on this PR as is.

Thank you for the explanation. I was wondering if it was something particular with the flaky test detector, or that I couldn't track flakiness from my local tests passing

kshi36 · 2025-11-20T19:40:02Z

Friendly ping @r0mant @bernardjkim @kopiczko

r0mant

3 line fix + 80 lines of tests. Love it!

rosstimothy · 2025-11-20T20:56:19Z

/excludeflake *

kshi36 · 2025-11-25T19:31:45Z

@rosstimothy Was the bypassing flaky test detector successful?

rosstimothy · 2025-11-25T20:03:49Z

@kshi36 you have to rerun the action for it to see the exclusion comment.

backport-bot-workflows · 2025-11-25T20:37:03Z

@kshi36 See the table below for backport results.

Branch	Result
branch/v17	Failed
branch/v18	Failed

…60982) * Fix kube discovery service poll interval * Add unit test, pass down clocks in discovery configs * Leverage synctest, pass down clocks properly * Refactor

…tly (#61792) * Fix kube discovery service `poll_interval` value not set correctly (#60982) * Fix kube discovery service poll interval * Add unit test, pass down clocks in discovery configs * Leverage synctest, pass down clocks properly * Refactor * Readd discovery_eks_test.go and new test * Refactor logger * Use NewClientset over deprecated NewSimpleClientset * Update license

…tly (#61791) * Fix kube discovery service `poll_interval` value not set correctly (#60982) * Fix kube discovery service poll interval * Add unit test, pass down clocks in discovery configs * Leverage synctest, pass down clocks properly * Refactor * Fix test * Fix test * Readd test coverage

…60982) * Fix kube discovery service poll interval * Add unit test, pass down clocks in discovery configs * Leverage synctest, pass down clocks properly * Refactor

Fix kube discovery service poll interval

b00d03b

kshi36 added backport/branch/v16 backport/branch/v17 backport/branch/v18 and removed backport/branch/v16 labels Nov 4, 2025

Add unit test, pass down clocks in discovery configs

7dae6de

kshi36 requested review from bernardjkim and r0mant November 12, 2025 01:18

kshi36 marked this pull request as ready for review November 12, 2025 01:18

github-actions bot added discovery size/sm labels Nov 12, 2025

github-actions bot requested review from kopiczko and rosstimothy November 12, 2025 01:19

rosstimothy reviewed Nov 12, 2025

View reviewed changes

Leverage synctest, pass down clocks properly

6a20931

kshi36 commented Nov 17, 2025

View reviewed changes

kshi36 requested a review from rosstimothy November 17, 2025 20:04

rosstimothy reviewed Nov 17, 2025

View reviewed changes

lib/srv/discovery/discovery_test.go Outdated Show resolved Hide resolved

lib/srv/discovery/discovery_eks_test.go Outdated Show resolved Hide resolved

lib/srv/discovery/discovery_test.go Show resolved Hide resolved

Refactor

37db27c

rosstimothy approved these changes Nov 17, 2025

View reviewed changes

Merge branch 'master' into kevin/discovery-poll-interval-fix

d4e239a

r0mant approved these changes Nov 20, 2025

View reviewed changes

public-teleport-github-review-bot bot removed request for bernardjkim and kopiczko November 20, 2025 20:50

bernardjkim approved these changes Nov 20, 2025

View reviewed changes

kshi36 enabled auto-merge November 20, 2025 22:27

kshi36 added this pull request to the merge queue Nov 25, 2025

Merged via the queue into master with commit de20a3d Nov 25, 2025
43 of 44 checks passed

kshi36 deleted the kevin/discovery-poll-interval-fix branch November 25, 2025 20:35

kshi36 mentioned this pull request Nov 25, 2025

[v18] Fix kube discovery service poll_interval value not set correctly #61791

Merged

kshi36 mentioned this pull request Nov 25, 2025

[v17] Fix kube discovery service poll_interval value not set correctly #61792

Merged

Conversation

kshi36 commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rosstimothy commented Nov 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kshi36 Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

rosstimothy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rosstimothy left a comment

Choose a reason for hiding this comment

Uh oh!

kshi36 commented Nov 17, 2025

Uh oh!

rosstimothy commented Nov 17, 2025

Uh oh!

kshi36 commented Nov 17, 2025

Uh oh!

kshi36 commented Nov 20, 2025

Uh oh!

r0mant left a comment

Choose a reason for hiding this comment

Uh oh!

rosstimothy commented Nov 20, 2025

Uh oh!

kshi36 commented Nov 25, 2025

Uh oh!

rosstimothy commented Nov 25, 2025

Uh oh!

Uh oh!

backport-bot-workflows bot commented Nov 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kshi36 commented Nov 4, 2025 •

edited

Loading