FairwindsOps/pluto

Replacement not available in the current cluster will be presented.

yktakaha4 opened this issue · 4 comments

Is your feature request related to a problem? Please describe.

This wonderful library has always helped me.

autoscaling/v2beta1.HorizontalPodAutoscaler on a v.1.22.0 cluster and received the following error in the detect-files on CI.

NAME NAMESPACE KIND VERSION REPLACEMENT DEPRECATED DEPRECATED REMOVED REMOVED IN  
sample-hpa sample HorizontalPodAutoscaler autoscaling/v2beta1 autoscaling/v2 true v1.22.0 false v1.25.0 

However, since HorizontalPodAutoscaler is enabled starting with v.1.23.0 clusters, so Replacement is not available.
https://kubernetes.io/docs/reference/using-api/deprecation-guide/#horizontalpodautoscaler-v125

Describe the solution you'd like

I want to incrementally upgrade to a valid resource in the current cluster.

Describe alternatives you've considered

As a way to suppress the error without affecting the schema or update operations in versions.yaml, how about presenting a version with the same Replacement within a k8s version range larger than the specified one?

For example, we present autoscaling/v2beta2.HorizontalPodAutoscaler as one of the Replacement choices for autoscaling/v2beta1.HorizontalPodAutoscaler
We can also maintain backward compatibility by changing the behavior only when certain options are set.

pluto/versions.yaml

Lines 236 to 259 in 89514e7

- version: autoscaling/v2beta1
kind: HorizontalPodAutoscaler
deprecated-in: v1.22.0
removed-in: v1.25.0
replacement-api: autoscaling/v2
component: k8s
- version: autoscaling/v2beta1
kind: HorizontalPodAutoscalerList
deprecated-in: v1.22.0
removed-in: v1.25.0
replacement-api: autoscaling/v2
component: k8s
- version: autoscaling/v2beta2
kind: HorizontalPodAutoscaler
deprecated-in: v1.23.0
removed-in: v1.26.0
replacement-api: autoscaling/v2
component: k8s
- version: autoscaling/v2beta2
kind: HorizontalPodAutoscalerList
deprecated-in: v1.23.0
removed-in: v1.26.0
replacement-api: autoscaling/v2
component: k8s

I also found the following Issue.
For the Ingress issue pointed out here, users can perform an incremental upgrade by presenting networking.k8s.io/v1beta1.Ingress for extensions/v1beta1.Ingress.
ref: #161

If this proposal is accepted, I would like to create a PR.

Additional context

This makes a lot of sense, but I'm still a bit unclear on the proposed implementation of this. Are you suggesting modifying the versions file to suggest autoscaling/v2beta2 as the replacement for v2beta1, rather than jumping directly to v2, and then adding another versions file entry for the upgrade from v2beta2 to v2?

Or are you suggesting a more complex behavior where the user would request "incremental versions" and then we add that to the versions spec? If that's the case, this is a much bigger (potentially breaking) change to the behavior of Pluto.

Thanks for your comments.

My idea is to present multiple values for REPLACEMENT when certain options (e.g. --show-deprecated-replacement) are set.

NAME NAMESPACE KIND VERSION REPLACEMENT DEPRECATED DEPRECATED REMOVED REMOVED IN  
sample-hpa sample HorizontalPodAutoscaler autoscaling/v2beta1 autoscaling/v2 true v1.22.0 false v1.25.0
sample-hpa sample HorizontalPodAutoscaler autoscaling/v2beta1 autoscaling/v2beta2 true v1.22.0 false v1.25.0

or

NAME NAMESPACE KIND VERSION REPLACEMENT DEPRECATED DEPRECATED REMOVED REMOVED IN  
sample-hpa sample HorizontalPodAutoscaler autoscaling/v2beta1 autoscaling/v2,autoscaling/v2beta2 true v1.22.0 false v1.25.0 

The following functions return multiple Versions or modify the value of Version.ReplacementAPI.
I don't think it is mandatory to add a new deprecated-versions item to values.yaml if the Version is generated in the function.

func (instance *Instance) checkVersion(stub *Stub) *Version {
for _, version := range instance.DeprecatedVersions {
// We allow empty kinds to deprecate whole APIs.
if version.Kind == "" || version.Kind == stub.Kind {
if version.Name == stub.APIVersion {
if version.Kind == "" {
version.Kind = stub.Kind
}
return &version
}
}
}
return nil
}

By the way, a more radical fix would be to add available-in to values.yaml to make the decision, but which would be better than what is being considered in this Issue?

Filling in the available-in from past k8s versions seems like a lot of work, but as a pure feature addition to pluto, it seems appropriate.
If it is acceptable to change the update operation of values.yaml, I am willing to create a PR this way.

I actually really like the idea to add a field to the versions.yaml file that introduces an available-in field. It definitely seems like a bit more work, but is a cleaner solution that should cover more use-cases.

This may be a major version change, but seems worth the effort.

Thanks.
I will try to create a PR using available-in.