Bug 1725981 - oc explain does not work well with full resource.group names
Summary: oc explain does not work well with full resource.group names
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: oc
Version: 4.2.0
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
: 4.8.0
Assignee: Jan Chaloupka
QA Contact: zhou ying
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-07-01 22:54 UTC by Ben Parees
Modified: 2021-07-27 22:32 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Feature: Allow explaining resources with full resource.group name. Either with oc explain or with kubectl explain command. Reason: In case two resources of a different group have the same resource name, it's practical to have oc/kubectl command automatically detect the group from provided resource string. Until now the group name part was considered as a field path and had no effect on specifying the group. Result: When a resource with full resource group name is specified, oc/kubectl tries to detect the correct group and select the one that corresponds with the provided group name the most.
Clone Of:
Environment:
Last Closed: 2021-07-27 22:32:19 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github kubernetes kubernetes pull 91295 0 None closed kubectl explain: detect resource group in case there are two or more groups discovered 2021-04-09 12:01:48 UTC
Github openshift kubernetes pull 608 0 None closed UPSTREAM: 91295: kubectl explain: detect resource group in case there… 2021-04-09 12:01:50 UTC
Github openshift oc pull 773 0 None closed Bump 4.8 kubernetes 1 21 0 beta 1 2021-04-09 12:02:06 UTC
Red Hat Product Errata RHSA-2021:2438 0 None None None 2021-07-27 22:32:36 UTC

Description Ben Parees 2019-07-01 22:54:13 UTC
Description of problem:
When a cluster has two identically named resources (in different groups) oc explain can't disambiguate them.


Version-Release number of selected component (if applicable):
Client Version: version.Info{Major:"4", Minor:"1+", GitVersion:"v4.1.0+373355c-908", GitCommit:"373355c0aa", GitTreeState:"clean", BuildDate:"2019-06-28T14:01:09Z", GoVersion:"go1.12.6", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"14+", GitVersion:"v1.14.0+bba922f", GitCommit:"bba922f", GitTreeState:"clean", BuildDate:"2019-06-29T04:14:37Z", GoVersion:"go1.12.5", Compiler:"gc", Platform:"linux/amd64"}



How reproducible:
always

Steps to Reproduce:
1. stand up a 4.2 cluster
2. run "oc explain authentications.operator.openshift.io"


Actual results:
error: field "operator" does not exist


Expected results:
an explanation of the authentications.operator.openshift.io resource

Additional info:
Running "oc explain authentications" isn't a solution because that gives me the result for "authentications.config.openshift.io" which is a different resource.

Comment 1 Ben Parees 2019-07-01 22:55:30 UTC
Michal indicated to me that he thought this worked so CCing him in case i missed something.

Comment 2 Xingxing Xia 2019-07-02 06:18:27 UTC
Should use `--api-version`, even for unique resource kind:
$ oc explain kubeapiservers.operator.openshift.io
error: field "operator" does not exist
$ oc explain kubeapiservers --api-version=operator.openshift.io/v1
KIND:     KubeAPIServer
VERSION:  operator.openshift.io/v1

DESCRIPTION:
...

Comment 3 Xingxing Xia 2019-07-02 06:21:49 UTC
Using short name without `--api-version` explains the resource of "default" api-version.

Comment 4 Ben Parees 2019-07-10 20:37:41 UTC
discussed this with david and he agreed that it would be better if "oc explain authentications.operator.openshift.io" just worked as one would expect.

Comment 5 Ben Parees 2019-07-10 20:38:46 UTC
(to be clear, --api-version does work as expected when used, this BZ is to improve the behavior so --api-version isn't needed if you just specify the full resource+groupname)

Comment 6 Xingxing Xia 2019-07-11 01:20:35 UTC
Sounds cool! :) Then it means it is expected to automatically tell if authentications.x.y.z represents field names or full resource+groupname

Comment 7 Maciej Szulik 2020-05-04 14:25:16 UTC
This bug hasn't had any activity in quite some time. Maybe the problem got
resolved, was a duplicate of something else, or became less pressing for some
reason - or maybe it's still relevant but just hasn't been looked at yet. As
such, we're closing this bug.

If you have further information on the current state of the bug, please add it
and reopen this bug. The information can be, for example, that the problem
still occurs, that you still want the feature, that more information is needed,
or that the bug is (for whatever reason) no longer relevant.

Comment 8 Ben Parees 2020-05-04 15:19:11 UTC
There is sufficient data on this bug to explain what goes wrong and what is expected (and agreement from you on what the right behavior might be).  What other data are you seeking?

If you think this might be fixed, then why not try it out, or send it to ON_QA and let them try it?

Closed insufficient data is the wrong resolution here.  And "closed, might be fixed" is (almost) never the right resolution.  Certainly not the right resolution when it's easy to check.

If you're not going to fix it, then "closed, won't fix" is the resolution you're looking for.

Or maybe this should be turned into an RFE.

Comment 9 Maciej Szulik 2020-05-04 16:35:56 UTC
Lemme have another look at it, moving to 4.5 for another review.

Comment 10 Jan Chaloupka 2020-05-12 10:57:30 UTC
`oc explain authentications.operator.openshift.io` does not work as expected since the command does not parse a group from the input string. Very long time ago the command supported '/' character to separate group from the rest of the string (see https://github.com/kubernetes/kubernetes/pull/15808). However, as of now "operator.openshift.io" is interpreted as a path in "authentications" resource.

Even with automatic recognition of the group, with the following two cases
1) group: a.b.c, field: d.e.f
2) group: a.b.c.d, field: e.f
the group string in "a.b.c.d.e.f" input string is still ambiguous.

Comment 11 Jan Chaloupka 2020-05-12 12:01:55 UTC
> (to be clear, --api-version does work as expected when used, this BZ is to improve the behavior so --api-version isn't needed if you just specify the full resource+groupname)

It does when invoked as `oc explain --api-version operator.openshift.io/v1 authentications`:
```
$ oc explain --api-version operator.openshift.io/v1 authentications
KIND:     Authentication
VERSION:  operator.openshift.io/v1

DESCRIPTION:
     Authentication provides information to configure an operator to manage
     authentication.

FIELDS:
...
```

Ben, the closest user solution we can implement is invoking `oc explain operator.openshift.io/authentications`. Which is equivalent to invoking `oc explain --api-version operator.openshift.io/v1 authentications` since you need to separate the group and kind anyway. Additionally, --api-version allows to explicitly specify the version in case authentications.operator.openshift.io gets upgraded to v2.

Instead, I suggest to update openshift docs to mention usage of --api-version option in this particular scenario. E.g. at https://docs.openshift.com/container-platform/4.4/cli_reference/openshift_cli/developer-cli-commands.html#explain or some place more suitable.

Will the suggested solution work for you?

Comment 12 Ben Parees 2020-05-12 12:47:54 UTC
> It does when invoked as `oc explain --api-version operator.openshift.io/v1 authentications`:

did you misread my statement as saying "--api-version doesn't work as expected"?  I agree that --api-version is the solution(workaround?) today.

> Will the suggested solution work for you?

having a solution that doesn't require me to specify the specific version and doesn't require me to explicitly type --api-version (oc explain operator.openshift.io/authentication) still seems slightly preferable but probably not a sufficient improvement to justify the work.  So if that's the best we can do, yeah i'm fine with solving this via better documentation instead.

That said, could we not attempt to do automatic group matching and if the match is ambiguous, come back to the user tell them they need to use --apiversion to specify the group explicitly?  I'm guessing the ambiguous cases are, in actual practice, rare.

Comment 13 Jan Chaloupka 2020-05-18 14:52:24 UTC
> did you misread my statement as saying "--api-version doesn't work as expected"? 

I am sorry, you are right. I misread your statement as "doesn't work".

> That said, could we not attempt to do automatic group matching and if the match is ambiguous, come back to the user tell them they need to use --apiversion to specify the group explicitly?  I'm guessing the ambiguous cases are, in actual practice, rare.

Lemme see how complicated it would be.

Comment 14 Jan Chaloupka 2020-05-20 14:25:06 UTC
Before `oc explain authentications.operator.openshift.io` starts displaying the resources:
- oc breaks the resource string into `authentications` and `operator.openshift.io`, the former as a resource to explain, the latter as a path in the resource
- oc asks discovery client to get all gvk triples that has `authentications` as a resources
- oc gets all gvk triples, in this case the following two: []schema.GroupVersionKind{schema.GroupVersionKind{Group:"config.openshift.io", Version:"v1", Kind:"Authentication"}, schema.GroupVersionKind{Group:"operator.openshift.io", Version:"v1", Kind:"Authentication"}}
- oc picks `config.openshift.io` as it has higher priority than `operator.openshift.io` (did not check how the priority is determined)

There's a possibility to use o.Mapper.KindsFor instead of o.Mapper.KindFor to get all available gvk triples and automatically detect some cases.

Upstream proposal for resolving this case: https://github.com/kubernetes/kubernetes/pull/91295

Comment 15 Jan Chaloupka 2020-05-20 14:29:30 UTC
Testing the code changes locally:

```
$ oc explain authentications
error: none of the discovereded groups [config.openshift.io/v1 operator.openshift.io/v1] matches the RESOURCE (authentications), update RESOURCE with additional group information
```

```
$ oc explain authentications.operator.openshift.io
KIND:     Authentication
VERSION:  operator.openshift.io/v1

DESCRIPTION:
     Authentication provides information to configure an operator to manage
     authentication.

FIELDS:
   apiVersion	<string>
     APIVersion defines the versioned schema of this representation of an
     object. Servers should convert recognized schemas to the latest internal
     value, and may reject unrecognized values. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources

   kind	<string>
     Kind is a string value representing the REST resource this object
     represents. Servers may infer this from the endpoint the client submits
     requests to. Cannot be updated. In CamelCase. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds

   metadata	<Object>
     Standard object's metadata. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata

   spec	<Object> -required-

   status	<Object>
```

```
$ oc explain authentications.operator.openshift.io.spec
KIND:     Authentication
VERSION:  operator.openshift.io/v1

RESOURCE: spec <Object>

DESCRIPTION:
     <empty>

FIELDS:
   logLevel	<string>
     logLevel is an intent based logging for an overall component. It does not
     give fine grained control, but it is a simple way to manage coarse grained
     logging choices that operators have to interpret for their operands.

   managementState	<string>
     managementState indicates whether and how the operator should manage the
     component

   observedConfig	<>
     observedConfig holds a sparse config that controller has observed from the
     cluster state. It exists in spec because it is an input to the level for
     the operator

   operatorLogLevel	<string>
     operatorLogLevel is an intent based logging for the operator itself. It
     does not give fine grained control, but it is a simple way to manage coarse
     grained logging choices that operators have to interpret for themselves.

   unsupportedConfigOverrides	<>
     unsupportedConfigOverrides holds a sparse config that will override any
     previously set options. It only needs to be the fields to override it will
     end up overlaying in the following order: 1. hardcoded defaults 2.
     observedConfig 3. unsupportedConfigOverrides
```

Comment 16 Michal Fojtik 2020-08-24 13:12:36 UTC
This bug hasn't had any activity in the last 30 days. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're marking this bug as "LifecycleStale" and decreasing the severity/priority. If you have further information on the current state of the bug, please update it, otherwise this bug can be closed in about 7 days. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

Comment 17 Ben Parees 2020-08-24 13:35:53 UTC
bug is in post but i don't see a PR linked, is there hope of moving it forward?

Comment 18 Jan Chaloupka 2020-09-10 14:23:08 UTC
I need more time to properly fix the issue upstream.

Comment 19 Jan Chaloupka 2020-11-13 12:08:21 UTC
I need more time to properly fix the issue upstream.

Comment 20 Jan Chaloupka 2021-01-15 11:04:40 UTC
Upstream PR updated, waiting for review

Comment 21 Jan Chaloupka 2021-03-08 09:46:12 UTC
Merged upstream, backporting to o/k: https://github.com/openshift/kubernetes/pull/608

Comment 22 Jan Chaloupka 2021-03-18 14:11:11 UTC
Waiting for the next oc rebase.

Comment 23 Maciej Szulik 2021-04-09 11:17:58 UTC
This merged in https://github.com/openshift/oc/pull/773

Comment 25 zhou ying 2021-04-14 03:06:53 UTC
[root@localhost roottest]# oc explain authentications.operator.openshift.io
KIND:     Authentication
VERSION:  operator.openshift.io/v1

DESCRIPTION:
     Authentication provides information to configure an operator to manage
     authentication.

FIELDS:
   apiVersion	<string>
     APIVersion defines the versioned schema of this representation of an
     object. Servers should convert recognized schemas to the latest internal
     value, and may reject unrecognized values. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources

   kind	<string>
     Kind is a string value representing the REST resource this object
     represents. Servers may infer this from the endpoint the client submits
     requests to. Cannot be updated. In CamelCase. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds

   metadata	<Object>
     Standard object's metadata. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata

   spec	<Object> -required-

   status	<Object>





[root@localhost roottest]# oc explain authentications.operator.openshift.io.spec
KIND:     Authentication
VERSION:  operator.openshift.io/v1

RESOURCE: spec <Object>

DESCRIPTION:
     <empty>

FIELDS:
   logLevel	<string>
     logLevel is an intent based logging for an overall component. It does not
     give fine grained control, but it is a simple way to manage coarse grained
     logging choices that operators have to interpret for their operands. Valid
     values are: "Normal", "Debug", "Trace", "TraceAll". Defaults to "Normal".

   managementState	<string>
     managementState indicates whether and how the operator should manage the
     component

   observedConfig	<>
     observedConfig holds a sparse config that controller has observed from the
     cluster state. It exists in spec because it is an input to the level for
     the operator

   operatorLogLevel	<string>
     operatorLogLevel is an intent based logging for the operator itself. It
     does not give fine grained control, but it is a simple way to manage coarse
     grained logging choices that operators have to interpret for themselves.
     Valid values are: "Normal", "Debug", "Trace", "TraceAll". Defaults to
     "Normal".

   unsupportedConfigOverrides	<>
     unsupportedConfigOverrides holds a sparse config that will override any
     previously set options. It only needs to be the fields to override it will
     end up overlaying in the following order: 1. hardcoded defaults 2.
     observedConfig 3. unsupportedConfigOverrides





[root@localhost roottest]#  oc version --client -o yaml 
clientVersion:
  buildDate: "2021-04-13T12:34:44Z"
  compiler: gc
  gitCommit: 783a74fd112c8f8bd12a7d6e7696d0eb49f09fe5
  gitTreeState: clean
  gitVersion: 4.8.0-202104131208.p0-783a74f
  goVersion: go1.16.1
  major: ""
  minor: ""
  platform: linux/amd64
releaseClientVersion: 4.8.0-0.nightly-2021-04-13-171608

Comment 30 errata-xmlrpc 2021-07-27 22:32:19 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Moderate: OpenShift Container Platform 4.8.2 bug fix and security update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2021:2438


Note You need to log in before you can comment on or make changes to this bug.