Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce InstancesState in the AppWorkload resrouce status #3781

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

georgethebeatle
Copy link
Member

Is there a related GitHub Issue?

#3755

What is this change about?

Today korifi looks directly at the pods running on the cluster, but once
the user provides a custom appworkload controller, this does not work
anymore, as it breaches the abstraction. Korifi should not rely on any
concorete implementation below the AppWorkload interface resource

To fix this we have introduced a new InstancesState field in the AppWorkload resource status.

@georgethebeatle georgethebeatle force-pushed the issues/3755-app-workload-state branch from 9d64dc8 to a992171 Compare February 5, 2025 15:34
tcdowney
tcdowney previously approved these changes Feb 5, 2025
Copy link
Member

@tcdowney tcdowney left a comment

Choose a reason for hiding this comment

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

This is a good idea 👍

Today korifi looks directly at the pods running on the cluster, but once
the user provides a custom appworkload controller, this does not work
anymore, as it breaches the abstraction. Korifi should not rely on any
concrete implementation below the AppWorkload interface resource.

Co-authored-by: Georgi Sabev <[email protected]>
@danail-branekov danail-branekov enabled auto-merge (rebase) February 6, 2025 15:16
@danail-branekov danail-branekov merged commit 6fde51a into main Feb 6, 2025
10 checks passed
@danail-branekov danail-branekov deleted the issues/3755-app-workload-state branch February 6, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants