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

Vpa to v1 cronjob #4887

Merged
merged 6 commits into from
May 16, 2022
Merged

Vpa to v1 cronjob #4887

merged 6 commits into from
May 16, 2022

Conversation

jbartosik
Copy link
Collaborator

@jbartosik jbartosik commented May 13, 2022 •
edited
Loading

This should fix #4886

Running all e2e tests would take a long time so I ran only part of the recommender test suite. All tests I run passed

jbartosik added 6 commits May 13, 2022 15:15
@jbartosik
Bump VPAs K8s dependencies to 1.24.0
f42600e 
We need to do this to use CronJobs from batch API v1
We need to use CronJobs from API v1 to work with K8s 1.25 (and for e2e to pass)
@jbartosik
Run go mod tidy
643744d 
After changing K8s deps
@jbartosik
Run go mod vendor
3af1001 
To update VPA vendor
@jbartosik
Switch CronJobs to v1 in VPA code
c79bdaf 
So it will work with K8s 1.25
@jbartosik
Move CronJob to v1 in e2e
7f20ae2 
So e2e will work with K8s 1.25
@jbartosik
Increase go version used to build VPA images
bc219ea 
One of our dependences (kubernetes-sigs/json) uses reflect.StructField.IsExported
which was added in go 1.17[1].

Also we should use newer go.

[1] https://go.dev/doc/go1.17#reflect
@k8s-ci-robot 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 May 13, 2022
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 13, 2022
@jbartosik jbartosik mentioned this pull request May 16, 2022
Closed
pbetkier
pbetkier approved these changes May 16, 2022
View reviewed changes
Copy link
Contributor

@pbetkier pbetkier left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Didn't look into vendor/, assuming it's fine.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jbartosik, pbetkier

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:

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

@pbetkier
Copy link
Contributor

/lgtm

@k8s-ci-robot
Copy link
Contributor

@pbetkier: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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/test-infra repository.

@krzysied
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 16, 2022
@k8s-ci-robot k8s-ci-robot merged commit e661d61 into kubernetes:master May 16, 2022
@jbartosik jbartosik deleted the vpa-to-v1-cronjob branch May 17, 2022 09:13
@zzzkl
Copy link

zzzkl commented Sep 7, 2022 •
edited
Loading

@jbartosik There isn't this fix in vpa 0.11.0 which cause some error log.
Is it possible to release a new verision of vpa.

E0907 01:19:58.621416 1 reflector.go:178] k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input/controller_fetcher/controller_fetcher.go:137: Failed to list *v1beta1.CronJob: the server could not find the requested resource

@jbartosik
Copy link
Collaborator Author

@zzzkl Yes. I'm working on it. Currently I'm waiting for approval on kubernetes/k8s.io#4148 (PR to promote VPA images from staging repo)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pbetkier pbetkier pbetkier approved these changes

@krzysied krzysied Awaiting requested review from krzysied

@MaciekPytel MaciekPytel Awaiting requested review from MaciekPytel

Assignees

@krzysied krzysied

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

All VPA e2e are failing
5 participants
@jbartosik @k8s-ci-robot @pbetkier @krzysied @zzzkl