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.

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

Add VPA version config-item #5361

Merged
merged 4 commits into from
Sep 15, 2022
Merged

Add VPA version config-item #5361

merged 4 commits into from
Sep 15, 2022

Conversation

demonCoder95
Copy link
Member

@demonCoder95 demonCoder95 commented Sep 15, 2022

In order to safely roll-out the latest version of the vertical-pod-autoscaler (rebase on latest upstream), this config-item is introduced as a flag to be able to restore to the legacy version in case of running into any issues.

More information in this issue: https://github.bus.zalan.do/teapot/issues/issues/3328

@demonCoder95
Copy link
Member Author

The vpa-updater has an outdated-flag in the container arguments:

unknown flag: --pod-lifetime-update-threshold
Usage of ./updater:
      --add-dir-header                                                  If true, adds the file directory to the header of the log messages
      --address string                                                  The address to expose Prometheus metrics. (default ":8943")
      --alsologtostderr                                                 log to standard error as well as files
      --evict-after-oom-threshold duration                              Evict pod that has only one container and it OOMed in less than
                                                                        		evict-after-oom-threshold since start. (default 10m0s)
      --eviction-rate-burst int                                         Burst of pods that can be evicted. (default 1)
unknown flag: --pod-lifetime-update-threshold
      --eviction-rate-limit float                                       Number of pods that can be evicted per seconds. A rate limit set to 0 or -1 will disable
                                                                        		the rate limiter. (default -1)
      --eviction-tolerance float                                        Fraction of replica count that can be evicted for update, if more than one pod can be evicted. (default 0.5)
      --in-recommendation-bounds-eviction-lifetime-threshold duration   Pods that live for at least that long can be evicted even if their request is within the [MinRecommended...MaxRecommended] range (default 12h0m0s)
      --kube-api-burst float                                            QPS burst limit when making requests to Kubernetes apiserver (default 10)
      --kube-api-qps float                                              QPS limit when making requests to Kubernetes apiserver (default 5)
      --kubeconfig string                                               Path to a kubeconfig. Only required if out-of-cluster.
      --log-backtrace-at traceLocation                                  when logging hits line file:N, emit a stack trace (default :0)
      --log-dir string                                                  If non-empty, write log files in this directory
      --log-file string                                                 If non-empty, use this log file
      --log-file-max-size uint                                          Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800)
      --logtostderr                                                     log to standard error instead of files (default true)
      --min-replicas int                                                Minimum number of replicas to perform update (default 2)
      --one-output                                                      If true, only write logs to their native severity level (vs also writing to each lower severity level)
      --pod-update-threshold float                                      Ignore updates that have priority lower than the value of this flag (default 0.1)
      --skip-headers                                                    If true, avoid header prefixes in the log messages
      --skip-log-headers                                                If true, avoid headers when opening log files
      --stderrthreshold severity                                        logs at or above this threshold go to stderr (default 2)
      --updater-interval duration                                       How often updater should run (default 1m0s)
      --use-admission-controller-status                                 If true, updater will only evict pods when admission controller status is valid. (default true)
  -v, --v Level                                                         number for the log level verbosity (default 0)
      --vmodule moduleSpec                                              comma-separated list of pattern=N settings for file-filtered logging
      --vpa-object-namespace string                                     Namespace to search for VPA objects. Empty means all namespaces will be used.

@demonCoder95
Copy link
Member Author

demonCoder95 commented Sep 15, 2022

The --pod-lifetime-update-threshold parameter was removed in favor of pod-update-threshold and in-recommendation-bounds-eviction-lifetime-threshold in release v0.10.0 of the vertical-pod-autoscaler. See the PR here kubernetes/autoscaler#3962

Simply removing the flag from the vpa-recommender container should fix this problem.

@demonCoder95
Copy link
Member Author

The flag is safe to remove since the previous default in the code [0] was 12h, same as the value we had set in our manifest [1]

[0] https://github.com/kubernetes/autoscaler/pull/3962/files#diff-2d1951e8948b2e86bd416513edadea4f736079ed4648203400b7e200f422e354L37

[1] b8a6cb7#diff-40f82f16b6e3b890123b895ba55211ab3c4c471f4ac060dda689908930677ae0L39

So, the change is backwards compatible.

@demonCoder95
Copy link
Member Author

👍

@demonCoder95
Copy link
Member Author

I think the PR is ready. Let's roll out! :)

@mikkeloscar
Copy link
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants