feat: Support PodRequests calculations for initContainers with restar… #569

sadath-12 · 2023-10-03T12:51:45Z

Fixes #479

Description

PodRequests calculations now also consider sidecar feature as introduced in v1.28 , hence also updating our k8s.io/client-go and k8s.io/api/core/v1 to latest version .
This pr is inspired from upstream k8s https://github.com/kubernetes/kubernetes/blob/e2afa175e4077d767745246662170acd86affeaf/pkg/api/v1/resource/helpers.go#L96

How was this change tested?

make presubmit

Ran this as a separate demo file comparing with the existing calculations done

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

pkg/utils/resources/resources.go

go.mod

sadath-12 · 2023-10-05T06:55:38Z

Also for test I would need to change podOptions interface a bit since with the current one it can't take multiple initContainers

sadath-12 · 2023-10-06T04:37:58Z

Will rework on this after latest k8s packages pr gets merged since it conflicts with this

go.sum

pkg/test/pods.go

pkg/utils/resources/resources.go

pkg/controllers/provisioning/provisioner_test.go

pkg/controllers/provisioning/nodepool_test.go

pkg/utils/resources/resources.go

pkg/controllers/provisioning/nodepool_test.go

sadath-12 · 2023-10-13T05:47:47Z

thank you @jonathan-innis , working on them

sadath-12 · 2023-10-13T13:06:10Z

Hi @jonathan-innis , Tried meeting those test requirements + additionally corrected logic and it passed the tests + tried to adapt to our existing Ceiling function structure + tried to make it as clean as possible. Please have a look

njtran

Just stopping by with a review. No need to block on me

pkg/test/pods.go

sadath-12 · 2023-10-18T04:35:14Z

Done

pkg/controllers/provisioning/nodepool_test.go

pkg/controllers/provisioning/provisioner_test.go

pkg/utils/resources/resources.go

coveralls · 2023-10-18T05:56:38Z

Pull Request Test Coverage Report for Build 6688219087

59 of 59 (100.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.07%) to 82.561%

Totals
Change from base Build 6672224362:	0.07%
Covered Lines:	9317
Relevant Lines:	11285

💛 - Coveralls

pkg/utils/resources/resources.go

pkg/utils/resources/resources_suite_test.go

pkg/utils/resources/resources_test.go

pkg/utils/resources/resources.go

ellistarn · 2023-10-23T21:15:33Z

Can we make sure we benchmark the scheduling performance before this is merged?

sadath-12 · 2023-10-24T08:25:18Z

Can we make sure we benchmark the scheduling performance before this is merged?

Sure!

pkg/utils/resources/resources.go

pkg/controllers/provisioning/scheduling/provisioner_instance_selection_test.go

.info

pkg/utils/resources/resources.go

pkg/controllers/provisioning/scheduling/provisioner_instance_selection_test.go

pkg/utils/resources/suite_test.go

sadath-12 · 2023-10-30T09:15:15Z

Applied @jonathan-innis

github-actions · 2023-11-13T12:01:40Z

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

jonathan-innis · 2023-11-20T16:58:13Z

@sadath-12 Looking for a rebase so that we can run the CI checks. PR is looking like it's in a good state and we can consider merging after we get a pass across the checks.

sadath-12 · 2023-11-22T07:36:15Z

Merged it @jonathan-innis , is it fine since it looked much easier

pkg/utils/resources/resources.go

pkg/utils/resources/suite_test.go

pkg/controllers/provisioning/scheduling/instance_selection_test.go

pkg/controllers/provisioning/nodepool_test.go

sadath-12 · 2023-11-25T13:26:49Z

Done

Signed-off-by: sadath-12 <[email protected]>

jonathan-innis

/lgtm

k8s-ci-robot · 2024-02-02T00:23:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jonathan-innis, sadath-12

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:

~~OWNERS~~ [jonathan-innis]

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

sadath-12 · 2024-02-02T04:13:14Z

Finally time has come

pkg/utils/resources/suite_test.go

jmdeal · 2024-02-02T05:13:22Z

A couple of small things, once those are taken care of this gets an lgtm from me.

jmdeal · 2024-02-02T17:50:37Z

/lgtm

jmdeal · 2024-02-02T17:51:16Z

/unhold

sadath-12 requested a review from a team as a code owner October 3, 2023 12:51

sadath-12 requested a review from njtran October 3, 2023 12:51

jonathan-innis reviewed Oct 5, 2023

View reviewed changes

pkg/utils/resources/resources.go Outdated Show resolved Hide resolved

pkg/utils/resources/resources.go Outdated Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

jonathan-innis reviewed Oct 13, 2023

View reviewed changes

njtran reviewed Oct 17, 2023

View reviewed changes

pkg/test/pods.go Outdated Show resolved Hide resolved

pkg/test/pods.go Outdated Show resolved Hide resolved

jonathan-innis force-pushed the sadath-dev-1 branch from 77e263c to 38c58b8 Compare October 18, 2023 05:35

jonathan-innis reviewed Oct 18, 2023

View reviewed changes

pkg/utils/resources/resources.go Outdated Show resolved Hide resolved

jonathan-innis reviewed Oct 19, 2023

View reviewed changes

jonathan-innis reviewed Oct 26, 2023

View reviewed changes

jonathan-innis reviewed Oct 30, 2023

View reviewed changes

.info Outdated Show resolved Hide resolved

pkg/utils/resources/resources.go Show resolved Hide resolved

pkg/utils/resources/resources.go Show resolved Hide resolved

pkg/utils/resources/resources.go Show resolved Hide resolved

pkg/utils/resources/resources.go Show resolved Hide resolved

jonathan-innis reviewed Oct 30, 2023

View reviewed changes

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

jonathan-innis removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 20, 2023

jonathan-innis reviewed Nov 24, 2023

View reviewed changes

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 25, 2023

sadath-12 and others added 8 commits February 1, 2024 16:14

test: added requests test

15894a4

Signed-off-by: sadath-12 <[email protected]>

test: resource merging

8a41148

Signed-off-by: sadath-12 <[email protected]>

Revert resource scheduling instance type selection test

067888b

added tests for resources

ce8fa38

Signed-off-by: sadath-12 <[email protected]>

format

831fce4

Signed-off-by: sadath-12 <[email protected]>

minorfixes

301db0d

Signed-off-by: sadath-12 <[email protected]>

fix lint

26d6af8

Signed-off-by: sadath-12 <[email protected]>

Test cleanup and gating on 1.29 version

f8f556d

jonathan-innis force-pushed the sadath-dev-1 branch from 6562ff8 to f8f556d Compare February 2, 2024 00:22

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 2, 2024

jonathan-innis approved these changes Feb 2, 2024

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 2, 2024

jmdeal reviewed Feb 2, 2024

View reviewed changes

pkg/utils/resources/suite_test.go Outdated Show resolved Hide resolved

pkg/utils/resources/suite_test.go Outdated Show resolved Hide resolved

pkg/utils/resources/suite_test.go Outdated Show resolved Hide resolved

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 2, 2024

jonathan-innis force-pushed the sadath-dev-1 branch 4 times, most recently from a531596 to d622740 Compare February 2, 2024 06:55

Remove redundant testing

c9638a4

jonathan-innis force-pushed the sadath-dev-1 branch from d622740 to c9638a4 Compare February 2, 2024 07:07

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 2, 2024

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 2, 2024

k8s-ci-robot merged commit 0e77b78 into kubernetes-sigs:main Feb 2, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support PodRequests calculations for initContainers with restar… #569

feat: Support PodRequests calculations for initContainers with restar… #569

sadath-12 commented Oct 3, 2023 •

edited

Loading

sadath-12 commented Oct 5, 2023

sadath-12 commented Oct 6, 2023

sadath-12 commented Oct 13, 2023

sadath-12 commented Oct 13, 2023

njtran left a comment

sadath-12 commented Oct 18, 2023

coveralls commented Oct 18, 2023 •

edited

Loading

ellistarn commented Oct 23, 2023

sadath-12 commented Oct 24, 2023

sadath-12 commented Oct 30, 2023 •

edited

Loading

github-actions bot commented Nov 13, 2023

jonathan-innis commented Nov 20, 2023

sadath-12 commented Nov 22, 2023

sadath-12 commented Nov 25, 2023

jonathan-innis left a comment

k8s-ci-robot commented Feb 2, 2024

sadath-12 commented Feb 2, 2024

jmdeal commented Feb 2, 2024

jmdeal commented Feb 2, 2024

jmdeal commented Feb 2, 2024

feat: Support PodRequests calculations for initContainers with restar… #569

feat: Support PodRequests calculations for initContainers with restar… #569

Conversation

sadath-12 commented Oct 3, 2023 • edited Loading

sadath-12 commented Oct 5, 2023

sadath-12 commented Oct 6, 2023

sadath-12 commented Oct 13, 2023

sadath-12 commented Oct 13, 2023

njtran left a comment

Choose a reason for hiding this comment

sadath-12 commented Oct 18, 2023

coveralls commented Oct 18, 2023 • edited Loading

Pull Request Test Coverage Report for Build 6688219087

💛 - Coveralls

ellistarn commented Oct 23, 2023

sadath-12 commented Oct 24, 2023

sadath-12 commented Oct 30, 2023 • edited Loading

github-actions bot commented Nov 13, 2023

jonathan-innis commented Nov 20, 2023

sadath-12 commented Nov 22, 2023

sadath-12 commented Nov 25, 2023

jonathan-innis left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 2, 2024

sadath-12 commented Feb 2, 2024

jmdeal commented Feb 2, 2024

jmdeal commented Feb 2, 2024

jmdeal commented Feb 2, 2024

sadath-12 commented Oct 3, 2023 •

edited

Loading

coveralls commented Oct 18, 2023 •

edited

Loading

sadath-12 commented Oct 30, 2023 •

edited

Loading