k8s 1.29 bump by kannon92 · Pull Request #1633 · kubernetes-sigs/kueue

kannon92 · 2024-01-22T22:55:28Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Update kubernetes to 0.29 api changes.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

I see two groups of failures:

field.NotSupported(field.NewPath("spec", "tolerations").Index(2).Child("effect"), corev1.TaintEffect("not-valid"), nil),

Error is "cannot infer t" and these errors are in pkg/webhooks/resourceflavor_webhook_test.go and pkg/webhooks/workload_webhooks_test.go.

And the pkg/controller/job/pod has the following error while running make test.

--- FAIL: TestReconciler_ErrorFinalizingPod (0.00s)
    /home/kehannon/Work/kueue/pkg/controller/jobs/pod/reconciler.go:233: "level"=2 "msg"="Reconciling Job" "job"="ns/pod" "gvk"="/v1, Kind=Pod"
    /home/kehannon/Work/kueue/pkg/controller/jobs/pod/pod_controller_test.go:2213: Expected reconcile error (-want,+got):
          any(
        - 	e"connection refused: connection refused",
          )
    /home/kehannon/Work/kueue/pkg/controller/jobs/pod/reconciler.go:233: "level"=2 "msg"="Reconciling Job" "job"="ns/pod" "gvk"="/v1, Kind=Pod"

Does this PR introduce a user-facing change?

NONE

netlify · 2024-01-22T22:55:34Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`ea8ef47`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/65b96943dd89420008276053

go.mod

tenzen-y · 2024-01-26T20:59:57Z

@kannon92 Hi! Are you still here?

I think we should include this in the next new minor version.
Since we will release the new minor version in the first week of February, could you complete this by its day?

kannon92 · 2024-01-26T21:38:25Z

I updated the PR but I need someone with Kueue experience to look into these errors.

I see two groups of failures:

field.NotSupported(field.NewPath("spec", "tolerations").Index(2).Child("effect"), corev1.TaintEffect("not-valid"), nil),
Error is "cannot infer t" and these errors are in pkg/webhooks/resourceflavor_webhook_test.go and pkg/webhooks/workload_webhooks_test.go.

And the pkg/controller/job/pod has the following error while running make test.

--- FAIL: TestReconciler_ErrorFinalizingPod (0.00s)
    /home/kehannon/Work/kueue/pkg/controller/jobs/pod/reconciler.go:233: "level"=2 "msg"="Reconciling Job" "job"="ns/pod" "gvk"="/v1, Kind=Pod"
    /home/kehannon/Work/kueue/pkg/controller/jobs/pod/pod_controller_test.go:2213: Expected reconcile error (-want,+got):
          any(
        - 	e"connection refused: connection refused",
          )
    /home/kehannon/Work/kueue/pkg/controller/jobs/pod/reconciler.go:233: "level"=2 "msg"="Reconciling Job" "job"="ns/pod" "gvk"="/v1, Kind=Pod"

Happy to resolve them if someone else can suggest a fix. Otherwise, I'm not sure how much progress I can make as I haven't done a lot of Kueue development lately.

kannon92 · 2024-01-26T21:43:18Z

Running make test:

=== Failed
=== FAIL: pkg/controller/jobs/pod TestReconciler_ErrorFinalizingPod (0.02s)
    reconciler.go:263: "level"=2 "msg"="Reconciling Job" "job"="ns/pod" "gvk"="/v1, Kind=Pod"
    pod_controller_test.go:2526: Expected reconcile error (-want,+got):
          any(
        -       e"connection refused: connection refused",
          )
    reconciler.go:263: "level"=2 "msg"="Reconciling Job" "job"="ns/pod" "gvk"="/v1, Kind=Pod"

=== Errors
pkg/webhooks/resourceflavor_webhook_test.go:112:23: cannot infer T (/home/kehannon/go/pkg/mod/k8s.io/apimachinery@v0.29.1/pkg/util/validation/field/errors.go:203:19)
pkg/webhooks/workload_webhook_test.go:263:23: cannot infer T (/home/kehannon/go/pkg/mod/k8s.io/apimachinery@v0.29.1/pkg/util/validation/field/errors.go:203:19)
pkg/webhooks/workload_webhook_test.go:349:23: cannot infer T (/home/kehannon/go/pkg/mod/k8s.io/apimachinery@v0.29.1/pkg/util/validation/field/errors.go:203:19)

@trasc At a quick glance you added

				field.NotSupported(field.NewPath("spec", "tolerations").Index(2).Child("effect"), corev1.TaintEffect("not-valid"), nil),

We are getting cannot infer T from usage of this in kube 1.29.

kannon92 · 2024-01-26T21:45:59Z

/cc @trasc

kannon92 · 2024-01-26T22:49:08Z

@kannon92 Hi! Are you still here?

I think we should include this in the next new minor version. Since we will release the new minor version in the first week of February, could you complete this by its day?

I don't think I'll be able to hit that february deadline as I'll be traveling.

mimowo · 2024-01-29T16:10:27Z

the first issue with cannot infer can probably be easily fixed by wrapping the nil with type []string(nil)?

I'm not sure about the missing reconcile error. Probably can take a look tomorrow.

kannon92 · 2024-01-30T05:24:52Z

the first issue with cannot infer can probably be easily fixed by wrapping the nil with type []string(nil)?

I'm not sure about the missing reconcile error. Probably can take a look tomorrow.

Thank you! That fix worked like a charm. Still got the reconcile error and it looks to also impact 1.29 e2e test

trasc · 2024-01-30T06:06:31Z

pkg/webhooks/resourceflavor_webhook_test.go

 					Effect:   corev1.TaintEffectNoSchedule,
 				}, ""),
-				field.NotSupported(field.NewPath("spec", "tolerations").Index(2).Child("effect"), corev1.TaintEffect("not-valid"), nil),
+				field.NotSupported(field.NewPath("spec", "tolerations").Index(2).Child("effect"), corev1.TaintEffect("not-valid"), []string{}),


Suggested change

field.NotSupported(field.NewPath("spec", "tolerations").Index(2).Child("effect"), corev1.TaintEffect("not-valid"), []string{}),

field.NotSupported(field.NewPath("spec", "tolerations").Index(2).Child("effect"), corev1.TaintEffect("not-valid"), []corev1.TaintEffect{}),

trasc · 2024-01-30T07:20:46Z

For the flaky e2e multikueue test we have #1659 under review.

For

    pod_controller_test.go:2526: Expected reconcile error (-want,+got):
          any(
        -       e"connection refused: connection refused",
          )
    reconciler.go:263: "level"=2 "msg"="Reconciling Job" "job"="ns/pod" "gvk"="/v1, Kind=Pod"

It looks like the TypeMeta of the object in

kueue/pkg/controller/jobs/pod/pod_controller_test.go

Lines 2492 to 2507 in b7fc93f

    
           	Update: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { 
        
           		gvk := obj.GetObjectKind().GroupVersionKind() 
        
           		if gvk.GroupVersion().String() == "v1" && gvk.Kind == "Pod" { 
        
           			defer func() { reqcount++ }() 
        
           			if reqcount == 0 { 
        
           				// return a connection refused error for the first update request. 
        
           				return errMock 
        
           			} 
        
           			if reqcount == 1 { 
        
           				// Exec a regular update operation for the second request 
        
           				return client.Update(ctx, obj, opts...) 
        
           			} 
        
           		} 
        
           		return client.Update(ctx, obj, opts...) 
        
           	}, 
        
           })

is not populated, you could :

a. get the GVK from the clients schema

                               kinds, _, err := client.Scheme().ObjectKinds(obj)
                               if err == nil {
                                       gvk = kinds[0]
                               }

b. do a type assertion instead _, isPod := obj.(*corev1.Pod)

mimowo · 2024-01-30T11:25:28Z

b. do a type assertion instead _, isPod := obj.(*corev1.Pod)

If this is just about unit tests, it is probably ok to go with b as simpler.

tenzen-y · 2024-01-30T21:17:28Z

For the flaky e2e multikueue test we have #1659 under review.

For
    pod_controller_test.go:2526: Expected reconcile error (-want,+got):
          any(
        -       e"connection refused: connection refused",
          )
    reconciler.go:263: "level"=2 "msg"="Reconciling Job" "job"="ns/pod" "gvk"="/v1, Kind=Pod"
It looks like the TypeMeta of the object in

kueue/pkg/controller/jobs/pod/pod_controller_test.go

Lines 2492 to 2507 in b7fc93f

Update: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.UpdateOption) error {

gvk := obj.GetObjectKind().GroupVersionKind()

if gvk.GroupVersion().String() == "v1" && gvk.Kind == "Pod" {

defer func() { reqcount++ }()

if reqcount == 0 {

// return a connection refused error for the first update request.

return errMock

}

if reqcount == 1 {

// Exec a regular update operation for the second request

return client.Update(ctx, obj, opts...)

}

}

return client.Update(ctx, obj, opts...)

},

})

is not populated, you could :

a. get the GVK from the clients schema
                               kinds, _, err := client.Scheme().ObjectKinds(obj)
                               if err == nil {
                                       gvk = kinds[0]
                               }
b. do a type assertion instead _, isPod := obj.(*corev1.Pod)

@trasc I guess that the root cause of the TypeMeta not being populated is kubernetes-sigs/controller-runtime#2633.

kannon92 · 2024-01-30T21:25:48Z

Pushed a fix for each. Thank you for looking into that.

kannon92 · 2024-01-30T21:49:08Z

/assign @trasc @tenzen-y
This is good to go now.

/retest

trasc

/lgtm

k8s-ci-robot · 2024-01-31T04:34:57Z

LGTM label has been added.

Details

Git tree hash: ffea7a787d1f05b97f48c6426da47254625ea516

tenzen-y

/approve

k8s-ci-robot · 2024-01-31T07:17:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kannon92, tenzen-y

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [tenzen-y]

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

* k8s 1.29 bump * fix for invalid values * update workloads * type assertion for pods * update with taint suggestion

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 22, 2024

k8s-ci-robot requested review from mimowo and trasc January 22, 2024 22:55

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 22, 2024

kannon92 force-pushed the bump-1-29 branch from f96a2cd to 33ba75e Compare January 22, 2024 22:59

kannon92 commented Jan 22, 2024

View reviewed changes

go.mod Outdated Show resolved Hide resolved

kannon92 force-pushed the bump-1-29 branch from 33ba75e to 83eef5f Compare January 22, 2024 23:01

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

kannon92 force-pushed the bump-1-29 branch from 83eef5f to 72e53ac Compare January 26, 2024 21:36

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

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

kannon92 added 2 commits January 29, 2024 14:50

k8s 1.29 bump

8d99fac

fix for invalid values

8b89978

kannon92 force-pushed the bump-1-29 branch from 72e53ac to 8b89978 Compare January 29, 2024 19:51

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

update workloads

b697c83

trasc reviewed Jan 30, 2024

View reviewed changes

kannon92 added 2 commits January 30, 2024 16:23

type assertion for pods

5e54fb2

update with taint suggestion

ea8ef47

k8s-ci-robot assigned tenzen-y and trasc Jan 30, 2024

trasc reviewed Jan 31, 2024

View reviewed changes

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

tenzen-y reviewed Jan 31, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 31, 2024

k8s-ci-robot merged commit 0b0961d into kubernetes-sigs:main Jan 31, 2024

k8s-ci-robot added this to the v0.6 milestone Jan 31, 2024

kannon92 added a commit to openshift-kannon92/kubernetes-sigs-kueue that referenced this pull request Nov 19, 2024

k8s 1.29 bump (kubernetes-sigs#1633)

9ce0bae

* k8s 1.29 bump * fix for invalid values * update workloads * type assertion for pods * update with taint suggestion

	field.NotSupported(field.NewPath("spec", "tolerations").Index(2).Child("effect"), corev1.TaintEffect("not-valid"), []string{}),
	field.NotSupported(field.NewPath("spec", "tolerations").Index(2).Child("effect"), corev1.TaintEffect("not-valid"), []corev1.TaintEffect{}),

Conversation

kannon92 commented Jan 22, 2024

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

netlify bot commented Jan 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Uh oh!

Uh oh!

tenzen-y commented Jan 26, 2024

Uh oh!

kannon92 commented Jan 26, 2024

Uh oh!

kannon92 commented Jan 26, 2024

Uh oh!

kannon92 commented Jan 26, 2024

Uh oh!

kannon92 commented Jan 26, 2024

Uh oh!

mimowo commented Jan 29, 2024

Uh oh!

kannon92 commented Jan 30, 2024

Uh oh!

trasc Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

kannon92 Jan 30, 2024

Choose a reason for hiding this comment

Uh oh!

trasc commented Jan 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mimowo commented Jan 30, 2024

Uh oh!

tenzen-y commented Jan 30, 2024

Uh oh!

kannon92 commented Jan 30, 2024

Uh oh!

kannon92 commented Jan 30, 2024

Uh oh!

trasc left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 31, 2024

Uh oh!

tenzen-y left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

netlify bot commented Jan 22, 2024 •

edited

Loading

trasc commented Jan 30, 2024 •

edited

Loading