[Draft] Observability and Reliability Batcher #1451

LPetro · 2024-07-20T04:28:43Z

Fixes #N/A

Description
This is a draft PR to start getting code and design feedback on the 'ORB' batch logger.

It extracts more-dynamic inputs to provisioning scheduling and logs salient aspects of them to a mounted PV. The data it logs is meant to assist in debugging provisioning and disruption/consolidation.

Milestones completed:

Scheduling inputs logged from provisioning scheduler.

Scheduling action metadata (from provisioning and disruption) sent to batcher and logged.

Scheduling inputs reduced to subset of fields for more concise logging.

Heaps built for batching via reconcile loops of the controller.

Diff functions built to compare these reduced fields for more efficient logging.

Logs serialized to protobuf.

Reconstruction functions to deserialize from protobuf.

Intended functionality not yet included:

Generalized set-up of PV, mounting and access of mount location (currently hard-coded to my dev S3 bucket and default '/data' mount location) and adding log retention policy.

Feature flagging the logging.

Command-line tool to pull logs back from PV and present them to the user.

Reconstruct scheduling input from logged baseline+diffs based on a time input to tool.

(Stretch Goal) Use reconstructed inputs to re-simulate a scheduling decision, to say "these inputs yield this set of nodeclaims" for verification/debugging.

How was this change tested?
Test writing in progress, not yet included. It does include a suite_test.go file, but this is just a blank template for now.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

linux-foundation-easycla · 2024-07-20T04:28:48Z

❌ - login: @LPetro / name: Louis Petro . The commit (ae3eb5c, f362a66, a0f0c80, 085fc5b, bcbc362, 29dc8d9, aaac6d0, d5438ba, ef62fb5, b0575c2, 26068cb, 516401a, 0638dc2, 1c3ce9e, 674d6b2, e63b53c, b4b3694, a762e48, 6699b02, 705595d, fbc47e9, 5a5b569, 09019cf, 443c070, 3492314, 339c35d, 5b975de, e3c62b4, fefe1d6, 529f823, 1172bce, 71bc5b6, e78e863, 6fce5fc, bf331b2) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

k8s-ci-robot · 2024-07-20T04:28:49Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LPetro
Once this PR has been reviewed and has the lgtm label, please assign jonathan-innis for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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 · 2024-07-20T04:28:52Z

Welcome @LPetro!

It looks like this is your first PR to kubernetes-sigs/karpenter 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/karpenter has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-07-20T04:28:52Z

Hi @LPetro. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

rschalo · 2024-07-23T19:03:01Z

pkg/controllers/controllers.go

+	SIHeap := orb.NewSchedulingInputHeap()
+	SMHeap := orb.NewSchedulingMetadataHeap()
+	p := provisioning.NewProvisioner(kubeClient, recorder, cloudProvider, cluster, SIHeap, SMHeap)


Wdyt about using a generic orb.Inject()

Or, I think what happens is here is that orb is instantiated, we can then call Inject() for a NewProvisioner and then pass a new orb controller to []controller.Controller{. See the metricsnode controller with its cluster param.

rschalo · 2024-07-23T19:28:34Z

pkg/controllers/provisioning/provisioner.go

 }

 func NewProvisioner(kubeClient client.Client, recorder events.Recorder,
 	cloudProvider cloudprovider.CloudProvider, cluster *state.Cluster,
+	schedulingInputHeap *orb.SchedulingInputHeap, schedulingMetadataHeap *orb.SchedulingMetadataHeap,


Why not have orb added like cloudProvider?

rschalo · 2024-07-23T19:29:06Z

pkg/controllers/orb/schedulinginputs.go

+)
+
+// These are the inputs to the scheduling function (scheduler.NewSchedule) which change more dynamically
+type SchedulingInput struct {


Is this just Input?

rschalo · 2024-07-23T19:29:20Z

pkg/controllers/orb/schedulingmetadata.go

+	//"google.golang.org/protobuf/proto"
+)
+
+type SchedulingMetadata struct {


Is this just metadata?

rschalo · 2024-07-23T20:33:50Z

pkg/controllers/orb/schedulinginputs.go

+func reducePodConditions(conditions []v1.PodCondition) []v1.PodCondition {
+	reducedConditions := []v1.PodCondition{}
+	for _, condition := range conditions {
+		reducedCondition := v1.PodCondition{
+			Type:    condition.Type,
+			Status:  condition.Status,
+			Reason:  condition.Reason,
+			Message: condition.Message,
+		}
+		reducedConditions = append(reducedConditions, reducedCondition)
+	}
+	return reducedConditions
+}


There isn't any filtering here, could we just use pod.Status.Conditions on schedulinginputs.go:125?

…arpenter

…tro functionality for PV tests and k8s built-in serialization.

…em all together.

…cture

…edulingInput construction.

…ter data to just what we want to log.

Added back (reduced) requirements to Offerings, and printed all for each IT (not just the first, whoops)

…er cycle.

…epEqual.

…ods struct.

…nexported type errors

… tracking for easier reconstruction later. Simplified some diff logic.

…nges. Also some code refactoring.

…ends to file for inspection. Fixed protobuf version I was using.

…uce before adding to heap.

…e representation, but I think they're required for reconstruction later. Added an empty test file based on Provisioning which I'll start populating soon.

…I definition, to ToStrings, to proto, to the proto-reconstruct functions and overhauled diff functions to have better wrappers for simplicity and robustness.

…nd associated changes. Updated SchedulingInput String function to just take the String representation of the protobuf. Iteration code clean-up.

k8s-ci-robot · 2024-08-13T06:48:36Z

PR needs rebase.

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.

github-actions · 2024-08-27T12:01:54Z

This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 20, 2024

k8s-ci-robot requested review from jackfrancis and tallaxes July 20, 2024 04:28

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 20, 2024

LPetro marked this pull request as draft July 20, 2024 04:29

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2024

rschalo reviewed Jul 23, 2024

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 30, 2024

LPetro added 13 commits August 6, 2024 16:59

Added ORB Batcher controller, very basic, just prints HelloWorld to K…

e63b53c

…arpenter

initial commit for ORB POCing in this branch

ae3eb5c

Set up the make apply settings to mount a pv to the Karpenter run. In…

339c35d

…tro functionality for PV tests and k8s built-in serialization.

Added a SchedulingInput concept and its queue, to test structuring th…

ef62fb5

…em all together.

Implemented the initial test pull-back down from PV back to data stru…

fbc47e9

…cture

Changed from queue to min heap, cleaned up some code, expanded on Sch…

bf331b2

…edulingInput construction.

Custom protobuf testing and internal k8s proto testing for mini-demo.

443c070

Changed orb package name and reorganized some files.

705595d

More testing for custom protobuffing.

a0f0c80

First pass at adding a reducing paradigm, reducing the K8S and Karpen…

085fc5b

…ter data to just what we want to log.

More reduce functions for Scheduling Inputs

3492314

Added back (reduced) requirements to Offerings, and printed all for each IT (not just the first, whoops)

Added logging logic for scheduling action metadata

0638dc2

Set up the diff code for SchedulingInput. Initial testing of it.

b4b3694

LPetro added 17 commits August 6, 2024 17:00

Added in heap for batching scheduling metadata together in one file p…

e3c62b4

…er cycle.

Updated Diff functionality with DeepEqual.

674d6b2

Changed some diff logic to prevent errors with unexported types in De…

71bc5b6

…epEqual.

Shifted from DeepEqual with StateNodes to defining own StateNodeWithP…

516401a

…ods struct.

Reduced what equality I was checking within InstanceType to prevent u…

29dc8d9

…nexported type errors

Changed from most-recent to baseline for our internal SchedulingInput…

6699b02

… tracking for easier reconstruction later. Simplified some diff logic.

Testing new Requirements equality functionality with InstanceType cha…

1c3ce9e

…nges. Also some code refactoring.

First pass at protobuffing fully custom with SchedulingInput

bcbc362

Set up proto for scheduling metadata and differences.

d5438ba

Added in deserialization test (for baseline file). Reconstructs and s…

09019cf

…ends to file for inspection. Fixed protobuf version I was using.

Some code clean up and rearranging. Fixed error where I forgot to red…

5a5b569

…uce before adding to heap.

Key and Operator in Requirements add a lot to the size of InstanceTyp…

aaac6d0

…e representation, but I think they're required for reconstruction later. Added an empty test file based on Provisioning which I'll start populating soon.

Added in cluster bindings, a getter method from Cluster, changes to S…

26068cb

…I definition, to ToStrings, to proto, to the proto-reconstruct functions and overhauled diff functions to have better wrappers for simplicity and robustness.

Cleaned up Diff function, added in Pod UID and Phase Condition info a…

5b975de

…nd associated changes. Updated SchedulingInput String function to just take the String representation of the protobuf. Iteration code clean-up.

Fetched upstream changes

b0575c2

Merge changes from orbpoc branch changes

f362a66

Fixes from rebasing with upstream/main

a762e48

LPetro force-pushed the orb branch from 4528439 to a762e48 Compare August 7, 2024 00:18

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2024

LPetro added 5 commits August 6, 2024 17:24

reverting kwok changes

e78e863

Rebasing changes and removing TODOs

1172bce

Kwok deployment changes for PV mounting. Reverted necessary changes.

6fce5fc

Pulled ORB tool out to provider-aws

fefe1d6

Testing still needs work, but added some framework

529f823

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 13, 2024

github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 27, 2024

github-actions bot added the lifecycle/closed label Sep 10, 2024

github-actions bot closed this Sep 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Draft] Observability and Reliability Batcher #1451

[Draft] Observability and Reliability Batcher #1451

LPetro commented Jul 20, 2024 •

edited

Loading

linux-foundation-easycla bot commented Jul 20, 2024 •

edited

Loading

k8s-ci-robot commented Jul 20, 2024

k8s-ci-robot commented Jul 20, 2024

k8s-ci-robot commented Jul 20, 2024

rschalo Jul 23, 2024

rschalo Jul 23, 2024

rschalo Jul 23, 2024

rschalo Jul 23, 2024

rschalo Jul 23, 2024

rschalo Jul 23, 2024 •

edited

Loading

k8s-ci-robot commented Aug 13, 2024

github-actions bot commented Aug 27, 2024

[Draft] Observability and Reliability Batcher #1451

[Draft] Observability and Reliability Batcher #1451

Conversation

LPetro commented Jul 20, 2024 • edited Loading

linux-foundation-easycla bot commented Jul 20, 2024 • edited Loading

k8s-ci-robot commented Jul 20, 2024

k8s-ci-robot commented Jul 20, 2024

k8s-ci-robot commented Jul 20, 2024

rschalo Jul 23, 2024

Choose a reason for hiding this comment

rschalo Jul 23, 2024

Choose a reason for hiding this comment

rschalo Jul 23, 2024

Choose a reason for hiding this comment

rschalo Jul 23, 2024

Choose a reason for hiding this comment

rschalo Jul 23, 2024

Choose a reason for hiding this comment

rschalo Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 13, 2024

github-actions bot commented Aug 27, 2024

LPetro commented Jul 20, 2024 •

edited

Loading

linux-foundation-easycla bot commented Jul 20, 2024 •

edited

Loading

rschalo Jul 23, 2024 •

edited

Loading