operator-sdk icon indicating copy to clipboard operation
operator-sdk copied to clipboard

operator-sdk and sub modules

Open filanov opened this issue 3 years ago • 10 comments

Type of question

Best practices --> General operator-related help -->

Question

What did you do?

I'm trying to run operator-sdk generate kustomize manifests --apis-dir api --input-dir ../config/manifests --output-dir ../config/manifests -q now api is a sub-module in the project so i need to run it from within the api directory but then it complain about the PROJECT file that doesn't exists in the local directory, as a workaround i copy the PROJECT file but i would prefer to give it as a parameter, from a short debug that i did i found that it's actually hard coded https://github.com/kubernetes-sigs/kubebuilder/blob/cf1575e08dff6c2b63d442b8fa6ec397fba64363/pkg/config/store/yaml/store.go#L65-L67 is there an option to make it configurable? i don't want to place the PROJECT file under API and have the configuration there as well because it will make a mess in the project and i have other commands that i would like to run outside of the api directory Or maybe there is a guide to how to work with operator-sdk when the project contain sub-modules.

Project structure is

root
  api (sub module)
  PROJECT
  config
  etc.

What did you expect to see?

Expect to have hacks that temporary copy the files.

What did you see instead? Under which circumstances?

at first i saw the sub-modules error

operator-sdk generate kustomize manifests --apis-dir api -q
-: pattern /home/mfilanov/work/go/src/github.com/openshift/assisted-service/api/...: main module (github.com/openshift/assisted-service) does not contain package github.com/openshift/assisted-service/api/common
FATA[0000] Error generating kustomize files: error getting ClusterServiceVersion base: error generating ClusterServiceVersion definitions metadata: one or more API packages had type errors 
make: *** [generate-bundle] Error 1

then after running the command from the directory i'm getting this error

operator-sdk generate kustomize manifests --apis-dir . --input-dir ../config/manifests --output-dir ../config/manifests -q
Error: error reading configuration: unable to load the configuration: unable to read "PROJECT" file: open PROJECT: no such file or directory

Environment

Operator type:

/language go

Kubernetes cluster type:

OpenShift

$ operator-sdk version

operator-sdk version operator-sdk version: "v1.10.1", commit: "5b69c2803890aa841fb361a4b9526dd251fe02e6", kubernetes version: "1.20.2", go version: "go1.16.6", GOOS: "linux", GOARCH: "amd64"

$ go version (if language is Go)

go version go1.17.5 linux/amd64

Additional context

filanov avatar Mar 06 '22 13:03 filanov

There's no reason (that we're aware of) that the first command should be failing - our best guess is that your api package is generated incorrectly or otherwise missing something. What command did you use to generate the api, and did you do anything weird with it?

jberkhahn avatar Mar 16 '22 18:03 jberkhahn

Standard controller generation

        controller-gen ${crd_options} rbac:roleName=assisted-service-manager-role paths="./..." output:rbac:dir=${controller_rbac_path} \
        webhook paths="./..." output:crd:artifacts:config=${controller_crd_path}/bases
        kustomize build ${controller_crd_path} > ${controller_crd_path}/resources.yaml
        controller-gen object:headerFile=${hack_boilerplate} paths="./..."

Nothing special

filanov avatar Mar 16 '22 19:03 filanov

We discussed this at the triage meeting on 3/28 and we have some concerns. We could add a flag to pass the PROJECT file to operator-sdk generate kustomize manifests, and it might be a simple fix if we added it for only that specific command and for your usecase here. But given that PROJECT is used for almost every SDK command, it would be a bit weird to implement it like that rather than making it passable for every SDK command, which will almost certainly cause strange things to happen when done with go submodules like you're doing here. What if you generated a controller from the api directory?

We don't really see a good way to fix this at this time, but we'd like to hear your thoughts. We also sent an email out to the mailing list here asking if anyone else has experienced anything similar.

jberkhahn avatar Mar 28 '22 19:03 jberkhahn

Hi @filanov,

Assuming that you created the project with SDK and you changed its default layout;

a) Could you share the link for your repo, so we can see the changes made in the default layout which caused this behaviour? Could you provide the code, so we can check and reproduce the issue? b) Could you please let us know more about what motivates you to make these changes? What problems/scenarios lead you to make these changes?

Cheers,

camilamacedo86 avatar Mar 28 '22 21:03 camilamacedo86

Hey, The repo: https://github.com/openshift/assisted-service The generation script: https://github.com/openshift/assisted-service/blob/9a8432287116458e0fbafdfc08311c06134a6450/hack/generate.sh#L133-L175

The main issue with the import from between different projects. In the past we had to import from a different project that had at lest 10 replaces in their go.mod file, sadly go modules didn't like that and i had to do a lot of tries and replaces in order to fix a conflict between the required packages versions. Because of those changes every other project that imported from our project had the same issue. So eventually we fixed the original project and reduced the replaces in our repo as well, but still we have few replaces that we need to have. So for a better use we separate the API into a different module, so it's really easy to import the api without getting all the other requirements in the project.

filanov avatar Mar 29 '22 10:03 filanov

I hope it make sense

filanov avatar Mar 29 '22 10:03 filanov

This has become a common challenge for operator projects. You make an operator in the hopes that people will use your CRDs. If those users are themselves also writing go code, of course they'll import your structs. But with the default sdk layout, that will drag in all the baggage of your operator's dependencies to mix with that other project's dependencies, and it becomes a big mess very quickly.

Putting your CRD structs in their own dedicated go module makes their use by other projects easy and low-impact.

This is a significant part of the reason that the k8s API has its own repo (and module) for example, separate from the controllers that implement the API.

While I think this issue is asking operator-sdk to support operator projects that separate their API into its own module, I suggest strongly considering making that the default project layout in the future.

mhrivnak avatar Mar 29 '22 13:03 mhrivnak

First, let's try to understand the use case:

I want to create a go module for my apis so that I can import it.

Now let's try to mock this scenario:

Let's use the Memcached sample: https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v3/memcached-operator.

a) Run go mod init inside the api directory b) Run make bundle

Then, we will check the error:

bin/controller-gen "crd:trivialVersions=true,preserveUnknownFields=false" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases

-: go build [github.com/example/memcached-operator/api/v1alpha1](http://github.com/example/memcached-operator/api/v1alpha1): no Go files in

Error: not all generators ran successfully

run `controller-gen crd:trivialVersions=true,preserveUnknownFields=false rbac:roleName=manager-role webhook paths=./... output:crd:artifacts:config=config/crd/bases -w` to see all available markers, or `controller-gen crd:trivialVersions=true,preserveUnknownFields=false rbac:roleName=manager-role webhook paths=./... output:crd:artifacts:config=config/crd/bases -h` for usage

make: *** [Makefile:85: manifests] Error 1

That means the controller-gen (https://github.com/kubernetes-sigs/controller-tools/tree/master/cmd/controller-gen) is raising an error and is not accepting that.

So, the first point here is we check if controller-gen could be able to work and allow it. That shows all that we need to do here to address your scenario.

Now, let's think about your specific scenario and request:

By looking at the source code:

  • The PROJECT file: https://github.com/openshift/assisted-service/blob/master/PROJECT (we can check that you only scaffold one API)
  • However, by looking at the repo: https://github.com/openshift/assisted-service/tree/master/api (we can check that you have apis from different groups. Am I right?)

TL'DR:

Then, that means you ought to use the following command to enable the support to multi-groups: operator-sdk edit --multigroup=true so that the tool could work with a different layout. That would scaffold dif paths to allow you to do that. For further information, see https://book.kubebuilder.io/migration/multi-group.html.

  • You can check a sample with the layout here: https://github.com/kubernetes-sigs/kubebuilder/tree/master/testdata/project-v3-multigroup.
  • To check the diff, you can compare with: https://github.com/kubernetes-sigs/kubebuilder/tree/master/testdata/project-v3.

By looking at the errors faced:

Is SDK or its command complaining about what you have a Go Mod inside of the api directory? NO So, the problem here does not seem related to the use or not submodules at all, see:

Error: Error generating kustomize files: error getting ClusterServiceVersion base: error generating ClusterServiceVersion definitions metadata: one or more API packages had type errors

That occurs because the command could not generate correctly the CSV based on the config/ data. It probably is caused by the fact that we do not have all APIs scaffolded in your project using the tool So, the specific manifests used, for example, to track the apis that the project has, are not updated, and we cannot generate the CSV. See, e.g., https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v3/memcached-operator/config/crd/kustomization.yaml#L1-L6.

Error "unable to read "PROJECT" file: open PROJECT: no such file or directory."

Here the problem is that the tool cannot look for the PROJECT file in the expected path.

In this way, this problem shows to be more related to the fact that you did not use the tool to do all scaffolds, to define that your layout should support multi groups properly and deviated too much from its default layout. However, you still want to take advantage of the tool helpers. Thus, you still want to use it to generate the bundle and are looking for a way to accommodate it with all changes made by skipping the PROJECT file or informing another path for it to move forward with the command and not face the issue.

Please, ensure that you check: https://sdk.operatorframework.io/docs/faqs/#can-i-customize-the-projects-initialized-with-operator-sdk

Therefore, regards your request/RFE:

As described above, to address your need, I do not think the change should be done in SDK. To support work with Go modules and generate the files, we need to change controller-gen.

However, see that:

  • The info of your PROJECT file is not correct and does not match with your project since you have not used the tool to do scaffolds.
  • The info in the config file also shows missing the required data since you did not use the tool. So that does not seem that your bundle would be appropriately built as we described above.

So, before we begin to think about solving the issues by adding flags that just bypass some internal code tool checks and pre-requirements (IMHO), we need first to think and understand:

  • How/when/why do we use the PROJECT file to generate the bundle?

Is for we look at the correct paths for the apis? For example, the path changes when we have multigroup: true (supports multigroup). Is for us know if the layout is from go/v2, go/v3, ansible/v1, etc. plugins? Could we ensure that the bundle is generated correctly without the info provided?

  • If we need the info provided in the PROJECT file for this operation: could we ignore it by making valid assumptions? Should it not be better than the flag? Should we warn the uses in this scenario?

Also, in your case, you make the command work without raising issues when you move the PROJECT file to the inside of the apis dir. However, that does not necessarily mean that the problem faced with the command is sorted out adequately, that the tool is generating the bundle with all data correctly, etc., well as expected.

I hope that can help us out. Thank you for your attention and time.

Conclusions (IHMO) a) we can check if we can change the controller gen b) but we should not add this flag. This does not seem to sort out the use case.

camilamacedo86 avatar Mar 30 '22 10:03 camilamacedo86

Hi folks, 

The next step here seems like make controller-gen work with go modules. Could someone check that? Have we had an issue with controller tools (https://github.com/kubernetes-sigs/controller-tools) already? 

camilamacedo86 avatar Apr 14 '22 17:04 camilamacedo86

We recently hit this issue in github.com/openstack-k8s-operators. Looks like operator-sdk-1.22.0 resolves this (likely because it bumps controller-tools to 0.9). I think you could mark this issue as having been fixed in tag 1.22.

dprince avatar Jul 08 '22 14:07 dprince

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot avatar Oct 07 '22 01:10 openshift-bot

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten /remove-lifecycle stale

openshift-bot avatar Nov 06 '22 09:11 openshift-bot

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen. Mark the issue as fresh by commenting /remove-lifecycle rotten. Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-bot avatar Dec 07 '22 00:12 openshift-bot

@openshift-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen. Mark the issue as fresh by commenting /remove-lifecycle rotten. Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci[bot] avatar Dec 07 '22 00:12 openshift-ci[bot]