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

CSV specDescriptors for type pulled from wrong GVK #4409

Closed
aharbis opened this issue Jan 20, 2021 · 7 comments · Fixed by #4445
Closed

CSV specDescriptors for type pulled from wrong GVK #4409

aharbis opened this issue Jan 20, 2021 · 7 comments · Fixed by #4445
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. language/go Issue is related to a Go operator project
Milestone

Comments

@aharbis
Copy link
Contributor

aharbis commented Jan 20, 2021

Bug Report

What did you do?

We currently have a single group, datapower.ibm.com, two versions: v1beta1 and v1beta2, and the CRDs are as follows:

  • datapower.ibm.com/v1beta1: DataPowerService
  • datapower.ibm.com/v1beta2: DataPowerService + DataPowerMonitor

In the v1beta1 DataPowerService, we had an embedded DataPowerMonitorSpec type defined, as this existed as a property within our DataPowerService CRD. In v1beta2 we lifted this spec out into its own CRD, but with a refactor of the properties to support a new business logic and implementation of that function.

Generating the manifests as follows:

$ make packagemanifests \
  VERSION=$version \
  CHANNEL=$channel \
  IS_DEFAULT_CHANNEL=1

What did you expect to see?

Base CSV (config/manifests/bases/${project}.clusterserviceversion.yaml) created / updated with specDescriptors and statusDescriptors from the appropriate GVK.

Versioned CSV (packagemanifests/${version}/${project}.clusterserviceversion.yaml) created / updated with specDescriptors and statusDescriptors from the appropriate GVK.

What did you see instead? Under which circumstances?

When I generate the CSV and package manifests, the v1beta2 DataPowerMonitor specDescriptors are for some reason being pulled from the embedded spec in the v1beta1 DataPowerService kind.

The v1beta2 DataPowerService specDescriptors also appear to be pulled from the v1beta1 version, because it still shows the embedded datapowerMonitor property.

As far as I can tell, the generator here is failing to differentiate between the GVKs, and the specDescriptors are getting jumbled together.

Environment

Operator type:

/language go

Kubernetes cluster type: OCP 4.6.x

$ operator-sdk version

operator-sdk version: "v1.0.1", commit: "4169b318b578156ed56530f373d328276d040a1b", kubernetes version: "v1.18.2", go version: "go1.13.15 linux/amd64", GOOS: "linux", GOARCH: "amd64"

$ go version (if language is Go)

go version go1.13.12 linux/amd64

$ kubectl version

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.5", GitCommit:"e6503f8d8f769ace2f338794c914a96fc335df0f", GitTreeState:"clean", BuildDate:"2020-06-26T03:47:41Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.0+7070803", GitCommit:"70708036fc265771f8d0a45598209018a8b9bd3e", GitTreeState:"clean", BuildDate:"2020-12-05T12:01:07Z", GoVersion:"go1.15.2", Compiler:"gc", Platform:"linux/amd64"}

Possible Solution

Additional context

I've been working on migrating our project from v0.18.2 to v0.19.4 and recently made the jump to v1.0.1 to pick up the CSV marker support. I was not seeing this issue / behavior on v0.18.2, so it appears to be isolated to the new operator-sdk generate kustomize manifests or operator-sdk generate packagemanifests implementations.

$ cat PROJECT
projectName: datapower-operator-sdk-migration
domain: ibm.com
layout: go.kubebuilder.io/v2
multigroup: true
repo: github.ibm.com/datapower/datapower-operator-sdk-migration
resources:
- group: datapower
  kind: DataPowerService
  version: v1beta1
- group: datapower
  kind: DataPowerService
  version: v1beta2
- group: datapower
  kind: DataPowerMonitor
  version: v1beta2
version: 3-alpha
plugins:
  go.sdk.operatorframework.io/v2-alpha: {}
@openshift-ci-robot openshift-ci-robot added the language/go Issue is related to a Go operator project label Jan 20, 2021
@aharbis
Copy link
Contributor Author

aharbis commented Jan 20, 2021

I will add that it looks like this isn't limited to specDescriptors. The description being pulled for our v1beta2 DataPowerService is coming from our v1beta1 DataPowerService types file.

Looking in spec.customresourcedefinitions.owned, of course.

@aharbis
Copy link
Contributor Author

aharbis commented Jan 22, 2021

I've narrowed down that buildCRDDescriptionFromType in internal/generate/clusterserviceversion/bases/definitions/crd.go is returning incorrect data. I checked the crd struct from line 73 below and found that at this point the description pulled was incorrect, as were the specDescriptors.

(incorrect meaning: for the v1beta2 DataPowerService GVK, v1beta1 DataPowerService metadata was pulled)

// Create definitions for kind types found under the collected roots.
definitionsByGVK := make(map[schema.GroupVersionKind]*descriptionValues)
for _, gvk := range gvks {
kindType, hasKind := g.types[gvk.Kind]
if !hasKind {
log.Warnf("Skipping CSV annotation parsing for API %s: type %s not found", gvk, gvk.Kind)
continue
}
crd, err := g.buildCRDDescriptionFromType(gvk, kindType)
if err != nil {
return err
}
definitionsByGVK[gvk] = &descriptionValues{
crd: crd,
}
}

@aharbis
Copy link
Contributor Author

aharbis commented Jan 22, 2021

Digging a little more, the problem may not be within buildCRDDescriptionFromType. It looks like the kindType being passed into this function (line 73 above) has the incorrect data, which would mean it's coming from this line:

kindType, hasKind := g.types[gvk.Kind]

@estroz estroz added this to the v1.5.0 milestone Jan 25, 2021
@estroz
Copy link
Member

estroz commented Jan 25, 2021

/kind bug
/assign

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 25, 2021
@aharbis
Copy link
Contributor Author

aharbis commented Jan 25, 2021

@estroz Regarding the milestone, just wondering if this bug fix would be back-ported to the prior release streams, or if we'd be looking at an upgrade to v1.5.0 to fix this?

@estroz
Copy link
Member

estroz commented Jan 25, 2021

@aharbis likely backported if you want it to be (as per the backport policy). It is in v1.5.0 because I don't expect to have the time to fix this until that release. If you have time to work on this earlier (or anyone else reading this), feel free to assign yourself.

@estroz
Copy link
Member

estroz commented Jan 28, 2021

@aharbis I actually did have time to work on this (#4445). I'll cherry-pick this back to v1.3 so it'll be in v1.3.1 and v1.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. language/go Issue is related to a Go operator project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants