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.

Sign up for GitHub

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

Jump to bottom

Add support for remote control plane to SLI exporter #273

Merged
merged 1 commit into from
Dec 9, 2024
Merged

Add support for remote control plane to SLI exporter #273

merged 1 commit into from
Dec 9, 2024

Conversation

Kidswiss
Copy link
Contributor

@Kidswiss Kidswiss commented Nov 27, 2024 •
edited
Loading

Summary

This adds support for remote control planes for the SLI exporter. It now
takes an additional KUBECONFIG env var to connect to the control plane.

Additionally to determine the state of the maintenance of the service
cluster, it connects to the local service cluster to reconcile on
UpgradeJob objects.

Also the SLI-Prober doesn't need any configuration about the services
anymore. It will detect these on startup.

Checklist

  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog
  • Update tests.
  • Link this PR to related issues.

@vshnbot vshnbot mentioned this pull request Nov 27, 2024
Merged
3 tasks
@Kidswiss Kidswiss assigned Kidswiss and unassigned Kidswiss Dec 3, 2024
@Kidswiss Kidswiss added the minor label Dec 3, 2024
@Kidswiss Kidswiss force-pushed the add/sli-exporter-control-plane-support branch from 19524bf to 9ff0abf Compare December 4, 2024 12:06
@Kidswiss Kidswiss requested review from a team, TheBigLee, wejdross and zugao and removed request for a team December 4, 2024 12:44
@Kidswiss Kidswiss requested review from a team, TheBigLee and zugao and removed request for TheBigLee, zugao and a team December 4, 2024 15:11
@Kidswiss Kidswiss force-pushed the add/sli-exporter-control-plane-support branch from 9ff0abf to 98b0a28 Compare December 5, 2024 10:14
zugao
zugao reviewed Dec 5, 2024
View reviewed changes
cmd/sliexporter.go Outdated Show resolved Hide resolved
zugao
zugao reviewed Dec 5, 2024
View reviewed changes
Copy link
Collaborator

@zugao zugao left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Some small nitpicks but I don't see where you connect to the control plane, where do you set KUBECONFIG? I see only SERVICE_ KUBECONFIG which is not even mandatory

@Kidswiss
Copy link
Contributor Author

Kidswiss commented Dec 6, 2024

Some small nitpicks but I don't see where you connect to the control plane, where do you set KUBECONFIG? I see only SERVICE_ KUBECONFIG which is not even mandatory

KUBECONFIG is read in the ctrl.GetConfigOrDie() function. As seen here: https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.19.3/pkg/client/config#GetConfigOrDie

@Kidswiss Kidswiss force-pushed the add/sli-exporter-control-plane-support branch from 70d65e5 to feccc65 Compare December 6, 2024 09:22
@zugao
Copy link
Collaborator

zugao commented Dec 6, 2024

As pointed out in the docs PR, does this PR also addresses the problem of XRs from different service clusters? We should make sure this works, otherwise it's gonna be a bug which will have to fix later on.

@Kidswiss
Copy link
Contributor Author

Kidswiss commented Dec 6, 2024

As pointed out in the docs PR, does this PR also addresses the problem of XRs from different service clusters? We should make sure this works, otherwise it's gonna be a bug which will have to fix later on.

Yes I need to add that logic, somehow I missed that completely. Might end up having multiple k8s clients after all.

@Kidswiss Kidswiss force-pushed the add/sli-exporter-control-plane-support branch from feccc65 to 26c59ee Compare December 6, 2024 12:27
@Kidswiss
Copy link
Contributor Author

Kidswiss commented Dec 6, 2024

As pointed out in the docs PR, does this PR also addresses the problem of XRs from different service clusters? We should make sure this works, otherwise it's gonna be a bug which will have to fix later on.

I came up with a simple logic to match all control plane composites to a local service cluster, could you have another look?

@Kidswiss Kidswiss requested a review from zugao December 6, 2024 12:28
zugao
zugao approved these changes Dec 6, 2024
View reviewed changes
Copy link
Collaborator

@zugao zugao left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/sliexporter/sli_reconciler/reconciler.go Outdated Show resolved Hide resolved
@Kidswiss
Add remote control plane support for SLI-exporter
36762a3 
This adds support for remote control planes for the SLI exporter. It now
takes an additional `KUBECONFIG` env var to connect to the control plane.

Additionally to determine the state of the maintenance of the service
cluster, it connects to the local service cluster to reconcile on
`UpgradeJob` objects.

Also the SLI-Prober doesn't need any configuration about the services
anymore. It will detect these on startup.
@Kidswiss Kidswiss force-pushed the add/sli-exporter-control-plane-support branch from 682657a to 36762a3 Compare December 6, 2024 14:41
@Kidswiss Kidswiss merged commit 93a71c4 into master Dec 9, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zugao zugao zugao approved these changes

@wejdross wejdross wejdross approved these changes

@TheBigLee TheBigLee Awaiting requested review from TheBigLee TheBigLee was automatically assigned from vshn/schedar

Assignees
No one assigned
Labels
minor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Kidswiss @zugao @wejdross