Skip to content
This repository has been archived by the owner on Jan 19, 2023. It is now read-only.

Custom Resource view doesn't align with kubectl #405

Closed
alanmeadows opened this issue Nov 6, 2019 · 7 comments
Closed

Custom Resource view doesn't align with kubectl #405

alanmeadows opened this issue Nov 6, 2019 · 7 comments
Labels
api bug Something isn't working

Comments

@alanmeadows
Copy link

alanmeadows commented Nov 6, 2019

The view octant provides for custom resources is limited in usefulness as it does not take advantage of the rich column data that kubectl can display defined within the CRD. My expectation is that there would be parity between these two views in terms of a listing of objects.

For instance:

image

vs:

alan@m3-io-demo-2:~/metal3-dev-env$ kubectl get baremetalhost -n metal3
NAME       STATUS   PROVISIONING STATUS   CONSUMER   BMC                         HARDWARE PROFILE   ONLINE   ERROR
master-0   OK       ready                            ipmi://192.168.111.1:6230   unknown            false    
worker-0   OK       ready                            ipmi://192.168.111.1:6231   unknown            true     
alan@m3-io-demo-2:~/metal3-dev-env$ 

This would be expected to have been provided by the CRD as printer columns, such as the case for the baremetalhosts.metal3.io below -- perhaps defaulting back to the view that is currently available on Octant when these are not defined in a CRD. However, in this case, the CRD owner has informed us of what is important in terms of columns already.

...
  name: baremetalhosts.metal3.io
  resourceVersion: "692"
  selfLink: /apis/apiextensions.k8s.io/v1/customresourcedefinitions/baremetalhosts.metal3.io
  uid: ebf2510d-fcf2-4e8b-a174-98187657989a
spec:
  conversion:
    strategy: None
  group: metal3.io
  names:
    kind: BareMetalHost
    listKind: BareMetalHostList
    plural: baremetalhosts
    shortNames:
    - bmh
    - bmhost
    singular: baremetalhost
  preserveUnknownFields: true
  scope: Namespaced
  versions:
  - additionalPrinterColumns:
    - description: Operational status
      jsonPath: .status.operationalStatus
      name: Status
      type: string
    - description: Provisioning status
      jsonPath: .status.provisioning.state
      name: Provisioning Status
      type: string
    - description: Consumer using this host
      jsonPath: .spec.consumerRef.name
      name: Consumer
      type: string
    - description: Address of management controller
      jsonPath: .spec.bmc.address
      name: BMC
      type: string
    - description: The type of hardware detected
      jsonPath: .status.hardwareProfile
      name: Hardware Profile
      type: string
    - description: Whether the host is online or not
      jsonPath: .spec.online
      name: Online
      type: string
    - description: Most recent error
      jsonPath: .status.errorMessage
      name: Error
      type: string
...
@wwitzel3 wwitzel3 added api enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Nov 24, 2019
@wwitzel3
Copy link
Contributor

@alanmeadows thanks for reporting this, this appears to be a bug.

If a CRD defines AdditionalPrinterColumns they should be rendering in our table view.

https://github.com/vmware-tanzu/octant/blob/master/internal/printer/customresource.go#L33 is the code in question.

@alanmeadows
Copy link
Author

alanmeadows commented Nov 24, 2019

@wwitzel3 Thanks!

Appreciate pointing out the code location. My brief take would be that per https://github.com/kubernetes/apiextensions-apiserver/blob/master/pkg/apis/apiextensions/types.go#L34 that while it supports "global" AdditionalPrinterColumns (crd.Spec.AdditionalPrinterColumns) it is not yet supporting version specific AdditionalPrinterColumns (crd.Spec.Versions[].AdditionalPrinterColumns)

I think https://github.com/kubernetes/apiextensions-apiserver/blob/master/pkg/apis/apiextensions/types.go#L192-L200 helps me understand what is going on at least.

@wwitzel3
Copy link
Contributor

@alanmeadows yep, that looks like the case to me as well, if you're up for it, and created a PR I'd be happy to review it and help get it merged in.

@alanmeadows
Copy link
Author

@GarySmith want to take a look at a PR for this?

@GarySmith
Copy link
Contributor

@alanmeadows Sure, I can do that

@GarySmith
Copy link
Contributor

It looks like it is more complicated than just not rendering version specific columns. Even global printer columns do not work. For example, load (with kubectl apply -f) the CRD, internal/printer/testdata/crd-additional-columns.yaml, and the corresponding CR, internal/printer/testdata/crd.yaml, and view these custom resources in the octant UI -- none of the additional columns are printed. This is ironic since the unit tests in customresource_test.go "verify" that exactly this same data is rendered correctly (it is not)..

Digging a little deeper, it appears that in the running product the CRD that is passed to the CustomResourceListHandler in customeresource.go is structured differently than is expected -- the crd.Spec.AdditionalPrinterColumns is an empty array, even with these "global" additionalPrinterColumns. Furthemore, crd.Spec.Versions[0].AdditionalPrinterColumns is being populated, except for the JSONPath field, which is empty. The empty JSONPath is a roadblock for rendering the data correctly.

(In the unit test, the CRD is formatted "correctly", with the crd.Spec.AdditionalPrinterColumns containing the proper information.

@wwitzel3
Copy link
Contributor

Thank you for looking at this @GarySmith . I followed the steps you mentioned, of using the testdata CRD and it indeed does not render the output table correctly and the tests do seem to suggest the renders are working.

I'm going to dig a bit deeper in to this, I'll report back what I find.

GarySmith added a commit to GarySmith/octant that referenced this issue Dec 12, 2019
Custom Resource Definitions may additionalPrinterColumns that are
specific to a give api version.  The CustomResourceHandler and
CustomResourceListHandler were only handling top-level
additionalPrinterColumns that apply to all columns regardless of
version.  This change handles version-specific additionalPrinterColumns.
This change addresses vmware-archive#405
GarySmith added a commit to GarySmith/octant that referenced this issue Dec 12, 2019
Custom Resource Definitions may additionalPrinterColumns that are
specific to a give api version.  The CustomResourceHandler and
CustomResourceListHandler were only handling top-level
additionalPrinterColumns that apply to all columns regardless of
version.  This change handles version-specific additionalPrinterColumns.
This change addresses vmware-archive#405

Signed-off-by: Gary Smith <garysmith123@gmail.com>
@bryanl bryanl closed this as completed Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants