remove legacy resource and kind checks from the CLI #19657
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k 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 |
openshift-ci-robot
added
the
approved
openshift-ci-robot
added
the
size/L
| batch.Kind("Job"): checkJobReadiness, | ||
| apps.Kind("StatefulSet"): checkStatefulSetReadiness, | ||
| routeapi.Kind("Route"): checkRouteReadiness, | ||
| {Group: "", Kind: "Build"}: checkBuildReadiness, |
There was a problem hiding this comment.
@bparees when are you going to stop supporting oapi?
There was a problem hiding this comment.
The moment someone tells me we're allowed to drop it.
There was a problem hiding this comment.
The moment someone tells me we're allowed to drop it.
The process endpoint no longer returns old oapi resources. Is the only use-case you have for this working with legacy masters and previously created things? Could you simply create a migration command to be run during 3.10 and remove this code in 3.11?
There was a problem hiding this comment.
is it possible we could accomplish this via conversion logic so the storage migration would just cover it? Adding a whole new migration command for a single release seems like overkill. (I assume you're suggesting a new "oc adm migrate templateinstances" subcommand)
There was a problem hiding this comment.
is it possible we could accomplish this via conversion logic so the storage migration would just cover it? Adding a whole new migration command for a single release seems like overkill. (I assume you're suggesting a new "oc adm migrate templateinstances" subcommand)
Possible, but I would not recommend doing so. Your old and new servers would be fighting with each other. I would point out HPA as an example of a thing with similar needs (ref needs updating) that created a separate command to avoid muddying the API specific knowledge baked into conversions. Conversions are more sensitive and broadly applied than a targeted migrate command. Stakes are much higher.
| t.Errorf("Expected type error.") | ||
| } | ||
|
|
||
| if _, err := legacyscheme.Registry.RESTMapper().KindFor(template.LegacyResource("processedtemplates").WithVersion("")); err != nil { |
There was a problem hiding this comment.
this static restmapper is gone in 3.11
| extensionsapi.Resource("deployments/rollback"), | ||
| appsapi.Resource("deployments/rollback"), | ||
| kapi.Resource("pods/attach"), | ||
| kapi.Resource("namespaces/finalize"), | ||
| {Group: "", Resource: "buildconfigs/instantiatebinary"}, |
There was a problem hiding this comment.
I think that authorization sticks around longer than we'll keep the admission endpoints.
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
1 similar comment
|
/retest |
The CLI no longer deals in oapi types. No need to test the legacy resources and kinds.