You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/02/01 16:06:14 UTC
[GitHub] [camel-k] phantomjinx opened a new pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
phantomjinx opened a new pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978
<!-- Description -->
Moves the CRDs to reside in the config directory in line with operator-sdk and kustomize configurations so removing those resource from the deploy directory. With move to using operator-sdk 1.0.0+ for CRD, CSV and bundle generation, minor changes and fixes have had to be implemented to keep artifacts in line with existing requirements but also to update them where necessary.
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
NONE
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] phantomjinx commented on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
phantomjinx commented on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-771926329
> It looks good overall, the main point is that it seems a different version of `controller-gen` is used, and produces `Any` types in the CRD schemas, which is not allowed in `apiextensions.k8s.io/v1`, and also drops some descriptions. I wonder what version of `controller-gen` is actually used? It seems to miss some of the Kubebuilder markers on the `Flow` and `JSON` types. If that's a newer version, it may be a matter of moving these annotations along the types hierarchy.
It was using controller-gen 0.4.0 which I had previously compiled locally. The binary uses [runtime/debug/BuildInfo](https://golang.org/pkg/runtime/debug/#BuildInfo) to determine the version number and for local compilations this is generated as 'devel'. Wrapping the module in an outer module allows the preferred version to be printed (see [here|https://github.com/golang/go/issues/29228#issuecomment-457390055].
So controller-gen version fixed, version upgraded to 0.4.1 and no more surprises in the CRDs :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti edited a comment on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-770985761
Wow, that's a piece of work! I like it also brings things like automated `createdAt` field 😉.
It looks good overall, the main point is that it seems a different version of `controller-gen` is used, and produces `Any` types in the CRD schemas, which is not allowed in `apiextensions.k8s.io/v1`, and also drops some descriptions. I wonder what version of `controller-gen` is actually used? It seems to miss some of the Kubebuilder markers on the `Flow` and `JSON` types.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-772686637
@phantomjinx thanks. Yes that inverted logic is a mistake of mine 🤦🏼! That looks much better now 👍🏼.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] phantomjinx commented on a change in pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
phantomjinx commented on a change in pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#discussion_r569420588
##########
File path: script/gen_crd.sh
##########
@@ -20,49 +20,52 @@ set -e
location=$(dirname $0)
apidir=$location/../pkg/apis/camel
-echo "Generating CRDs..."
-
cd $apidir
-$CONTROLLER_GEN crd paths=./... output:crd:artifacts:config=false output:crd:dir=../../../deploy/crds crd:crdVersions=v1
+$CONTROLLER_GEN crd paths=./... output:crd:artifacts:config=../../../config/crd/bases output:crd:dir=../../../config/crd/bases crd:crdVersions=v1
-# cleanup
-rm -r ./config
+# cleanup working diretory in $apidir
Review comment:
👍🏼
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti edited a comment on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-770985761
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] phantomjinx edited a comment on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
phantomjinx edited a comment on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-772515578
@astefanutti
Added an extra commit that fixes some [incorrect logic](https://github.com/apache/camel-k/pull/1978/files#diff-9aecbcc15d64c3e7a146ee5776a35e65b45fb72461e370c4e295dd90f6d45262L342), identifying Darwin and the use of `sed`. It looks like the wrong way around & am guessing your running mac? In which case, could you test to see if you agree?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] phantomjinx edited a comment on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
phantomjinx edited a comment on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-771926329
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-774966636
Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] phantomjinx commented on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
phantomjinx commented on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-773981464
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] phantomjinx commented on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
phantomjinx commented on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-771926329
> It looks good overall, the main point is that it seems a different version of `controller-gen` is used, and produces `Any` types in the CRD schemas, which is not allowed in `apiextensions.k8s.io/v1`, and also drops some descriptions. I wonder what version of `controller-gen` is actually used? It seems to miss some of the Kubebuilder markers on the `Flow` and `JSON` types. If that's a newer version, it may be a matter of moving these annotations along the types hierarchy.
It was using controller-gen 0.4.0 which I had previously compiled locally. The binary uses [runtime/debug/BuildInfo](https://golang.org/pkg/runtime/debug/#BuildInfo) to determine the version number and for local compilations this is generated as 'devel'. Wrapping the module in an outer module allows the preferred version to be printed (see [here|https://github.com/golang/go/issues/29228#issuecomment-457390055].
So controller-gen version fixed, version upgraded to 0.4.1 and no more surprises in the CRDs :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti edited a comment on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-770985761
Wow, that's a piece of work! I like it also brings things like automated `createdAt` field 😉.
It looks good overall, the main point is that it seems a different version of `controller-gen` is used, and produces `Any` types in the CRD schemas, which is not allowed in `apiextensions.k8s.io/v1`, and also drops some descriptions. I wonder what version of `controller-gen` is actually used?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti merged pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti merged pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#discussion_r569382999
##########
File path: script/gen_crd.sh
##########
@@ -20,49 +20,52 @@ set -e
location=$(dirname $0)
apidir=$location/../pkg/apis/camel
-echo "Generating CRDs..."
-
cd $apidir
-$CONTROLLER_GEN crd paths=./... output:crd:artifacts:config=false output:crd:dir=../../../deploy/crds crd:crdVersions=v1
+$CONTROLLER_GEN crd paths=./... output:crd:artifacts:config=../../../config/crd/bases output:crd:dir=../../../config/crd/bases crd:crdVersions=v1
-# cleanup
-rm -r ./config
+# cleanup working diretory in $apidir
Review comment:
nit: `diretory` -> `directory`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-770985761
Wow, that's a piece of work! I like it also bring thing like automated `createdAt` field 😉.
It looks good overall, the main point is that it seems a different version of `controller-gen` is used, and produces `Any` types in the CRD schemas, which is not allowed in `apiextensions.k8s.io/v1`, and also drops some descriptions. I wonder what version of `controller-gen` is actually used?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] phantomjinx commented on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
phantomjinx commented on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-772515578
@astefanutti
Added an extra commit that fixes some [incorrect logic](https://github.com/apache/camel-k/pull/1978/files#diff-9aecbcc15d64c3e7a146ee5776a35e65b45fb72461e370c4e295dd90f6d45262L342), identifying Darwn and the use of `sed`. It looks like the wrong way around & am guessing your running mac? In which case, could you test to see if you agree?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti edited a comment on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-770985761
Wow, that's a piece of work! I like it also brings things like automated `createdAt` field 😉.
It looks good overall, the main point is that it seems a different version of `controller-gen` is used, and produces `Any` types in the CRD schemas, which is not allowed in `apiextensions.k8s.io/v1`, and also drops some descriptions. I wonder what version of `controller-gen` is actually used? It seems to miss some of the Kubebuilder markers on the `Flow` and `JSON` types. If that's a newer version, it may be a matter of moving these annotations along the types hierarchy.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on pull request #1978: Refactors the deploy directory to make config directory single source of CRD truth
Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #1978:
URL: https://github.com/apache/camel-k/pull/1978#issuecomment-772479429
Thanks @phantomjinx, that fixes it 👍🏼.
I've checked downstream, and the build calls the `embed_resources.sh` script directly, so it'll have to be updated to use `go generate` instead. The product Camel catalog generation should be alright, as the `deploy` directory has been kept with the community catalog.
Once CI passes, it should be good to go, and we'll update downstream build.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org