Skip to content

add option to do server side apply#113

Open
jwang99 wants to merge 1 commit intoreddit:mainfrom
jwang99:DATAPLAT-684/add-server-side-apply
Open

add option to do server side apply#113
jwang99 wants to merge 1 commit intoreddit:mainfrom
jwang99:DATAPLAT-684/add-server-side-apply

Conversation

@jwang99
Copy link
Copy Markdown

@jwang99 jwang99 commented May 4, 2026

💸 TL;DR

Adding an option to do server-side apply, in addition to merge-patch (default) and Update. This is to enable the achilles-flink-controller to fully manage its own fields without overwriting fields that are managed by another controller, the flink kubernetes operator.

Jira

🧪 Testing Steps / Validation

Added unit tests

✅ Checks

  • CI tests (if present) are passing
  • Adheres to code style for repo
  • Contributor License Agreement (CLA) completed if not a Reddit employee

Copy link
Copy Markdown

@william-richard william-richard left a comment

Choose a reason for hiding this comment

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

LGTM, but I don't feel quite qualified enough to approve

@william-richard william-richard requested a review from harveyxia May 7, 2026 14:28
Copy link
Copy Markdown
Collaborator

@harveyxia harveyxia left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding this! We should test e2e in a staging (or other high fidelity) environment before cutting a release

Comment thread pkg/io/applicator.go
for _, managedFields := range current.GetManagedFields() {
// we're doing a client-side apply, so we assume we own all fields even if the manager is not our own.
// in other words, no need to ensure that managedFields.Manager == a.managerName
// TODO: we should explore using server-side apply
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants