Skip to content

Conversation

08volt
Copy link
Contributor

@08volt 08volt commented Jul 9, 2025

This pull request introduces a new --read-only flag to run the ingress-gce controllers in a non-mutating, observational mode.

Summary

When the read-only flag is enabled, the controllers will not execute any mutating API calls (e.g., create, update, delete) against the cloud provider.

This change touches the following controllers:

  • Instance Groups
  • L4 ILB and L4 NetLB
  • Network Endpoint Group (NEG)
  • Private Service Connect (PSC)

Testing

Comprehensive tests have been added for each controller to ensure that no mutating operations are performed when read-only mode is active. Existing test suites have been parameterized to run in both standard and read-only modes, asserting that resources are not created or modified in the latter.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 9, 2025
@k8s-ci-robot k8s-ci-robot requested review from aojea and sawsa307 July 9, 2025 16:03
@k8s-ci-robot
Copy link
Contributor

Hi @08volt. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 9, 2025
@08volt 08volt changed the title Read-Only mode for controllers Read-Only Mode Jul 9, 2025
@TortillaZHawaii
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 10, 2025
@08volt 08volt force-pushed the read-only-glbc-1 branch 2 times, most recently from a43038f to af16c2b Compare July 10, 2025 09:23
@08volt
Copy link
Contributor Author

08volt commented Jul 10, 2025

/retest

@08volt 08volt force-pushed the read-only-glbc-1 branch from af16c2b to 1d28aee Compare July 10, 2025 10:26
@08volt
Copy link
Contributor Author

08volt commented Jul 10, 2025

/retest

1 similar comment
@08volt
Copy link
Contributor Author

08volt commented Jul 10, 2025

/retest

@08volt 08volt force-pushed the read-only-glbc-1 branch from 1d28aee to 3d04976 Compare July 15, 2025 13:17
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 17, 2025
@08volt 08volt force-pushed the read-only-glbc-1 branch from 3d04976 to ffbd665 Compare July 17, 2025 14:30
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 17, 2025
@08volt
Copy link
Contributor Author

08volt commented Jul 17, 2025

@FelipeYepez

Copy link
Contributor

@FelipeYepez FelipeYepez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please verify we add the read only mode only in the places we need it. Review all controllers and ask for review when done

@08volt 08volt force-pushed the read-only-glbc-1 branch from 1ef51e6 to 994f93c Compare July 22, 2025 09:20
@08volt
Copy link
Contributor Author

08volt commented Jul 22, 2025

/retest

@08volt 08volt force-pushed the read-only-glbc-1 branch 4 times, most recently from b03d134 to b50ed0d Compare July 22, 2025 14:18
@08volt 08volt force-pushed the read-only-glbc-1 branch 5 times, most recently from 57452b4 to 4a2900a Compare July 22, 2025 15:00
@08volt
Copy link
Contributor Author

08volt commented Jul 23, 2025

/retest

@08volt 08volt force-pushed the read-only-glbc-1 branch from 4a2900a to 56807e2 Compare July 23, 2025 14:08
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2025
@08volt 08volt force-pushed the read-only-glbc-1 branch 2 times, most recently from 5f719e3 to e823b0d Compare July 24, 2025 08:42
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 24, 2025
@08volt 08volt force-pushed the read-only-glbc-1 branch from e823b0d to 8bf4891 Compare July 24, 2025 08:58
@08volt 08volt force-pushed the read-only-glbc-1 branch from 8bf4891 to 3222987 Compare July 24, 2025 09:14
@mmamczur
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 20, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 08volt, mmamczur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 20, 2025
@k8s-ci-robot k8s-ci-robot merged commit 40aad96 into kubernetes:master Aug 20, 2025
5 checks passed
Copy link
Member

@swetharepakula swetharepakula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting on a closed PR, but we should convert this into metrics on the API calls instead of logs.

Also is there a read out of what this would look like? Will the controller keep erroring on its reads (because no writes have been made)?

@swetharepakula
Copy link
Member

Actually diving deeper, this doesn't feel like read only mode. This is more just logging when a sync is triggered

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants