You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2020/06/08 02:09:11 UTC
[GitHub] [incubator-yunikorn-k8shim] HuangTing-Yao opened a new pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
HuangTing-Yao opened a new pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134
Some code are generated by code-generator `release-1.14`
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] kingamarton commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r441311647
##########
File path: Makefile
##########
@@ -165,6 +165,16 @@ push: image
docker push ${REGISTRY}/yunikorn:scheduler-${VERSION}
docker push ${REGISTRY}/yunikorn:admission-${VERSION}
+#Generate the CRD code with code-generator (release-1.14)
+
+# If you want to re-run the code-generator to generate code,
+# Please make sure the directory structure must be the example.
+# ex: github.com/apache/incubator-yunikorn-k8shim
Review comment:
Thank you for the explanation. It makes sense
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter edited a comment on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bdacf71ac449ad54fe8e40ef7086b35e00af5c46&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
=======================================
Coverage 52.89% 52.89%
=======================================
Files 32 32
Lines 3025 3025
=======================================
Hits 1600 1600
Misses 1364 1364
Partials 61 61
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...c4234b6](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] yangwwei commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r436445953
##########
File path: pkg/apis/yunikorn.apache.org/v1alpha1/type.go
##########
@@ -0,0 +1,72 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package v1alpha1
+
+import (
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+type Application struct {
+ metav1.TypeMeta `json:",inline"`
+ metav1.ObjectMeta `json:"metadata"`
+
+ Spec ApplicationSpec `json:"spec"`
+ Status ApplicationStatus `json:"status"`
+}
+
+//Spec part
+
+type ApplicationSpec struct {
+ MinMember int32 `json:"minMember"`
+ Queue string `json:"queue"`
+ MaxPendingSeconds int32 `json:"maxPendingSeconds,omitempty"`
+}
+
+//Status part
+
+type ApplicationStateType string
+
+const (
+ NewState ApplicationStateType = ""
+ SubmittedState ApplicationStateType = "SUBMITTED"
+ RunningState ApplicationStateType = "RUNNING"
+ CompletedState ApplicationStateType = "COMPLETED"
+ FailedState ApplicationStateType = "FAILED"
+ FailedSubmissionState ApplicationStateType = "SUBMISSION_FAILED"
+ PendingRerunState ApplicationStateType = "PENDING_RERUN"
+ InvalidatingState ApplicationStateType = "INVALIDATING"
+ SucceedingState ApplicationStateType = "SUCCEEDING"
+ FailingState ApplicationStateType = "FAILING"
+ UnknownState ApplicationStateType = "UNKNOWN"
+)
+
+type ApplicationStatus struct {
+ AppStatus ApplicationStateType `json:"applicationState,omitempty"`
Review comment:
I think we need to include 2 more fields for app status:
```
type ApplicationStatus struct {
State ApplicationStateType
Message string // some arbitrary message to explain what happens
LastUpdate time // last app state update time
}
```
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# generate the code with:
+# --output-base because this script should also be able to run inside the vendor dir of
+# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
+# instead of the $GOPATH directly. For normal projects this can be dropped.
+GO111MODULE=off
Review comment:
We should enable go mod, I am expecting this value to be true.
All of our repos are managed by go mod, with GO111MODULE set to true.
##########
File path: pkg/apis/yunikorn.apache.org/v1alpha1/type.go
##########
@@ -0,0 +1,72 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package v1alpha1
+
+import (
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+type Application struct {
+ metav1.TypeMeta `json:",inline"`
+ metav1.ObjectMeta `json:"metadata"`
+
+ Spec ApplicationSpec `json:"spec"`
+ Status ApplicationStatus `json:"status"`
+}
+
+//Spec part
+
+type ApplicationSpec struct {
+ MinMember int32 `json:"minMember"`
+ Queue string `json:"queue"`
+ MaxPendingSeconds int32 `json:"maxPendingSeconds,omitempty"`
+}
+
+//Status part
+
+type ApplicationStateType string
+
+const (
+ NewState ApplicationStateType = ""
+ SubmittedState ApplicationStateType = "SUBMITTED"
+ RunningState ApplicationStateType = "RUNNING"
+ CompletedState ApplicationStateType = "COMPLETED"
+ FailedState ApplicationStateType = "FAILED"
+ FailedSubmissionState ApplicationStateType = "SUBMISSION_FAILED"
+ PendingRerunState ApplicationStateType = "PENDING_RERUN"
+ InvalidatingState ApplicationStateType = "INVALIDATING"
+ SucceedingState ApplicationStateType = "SUCCEEDING"
+ FailingState ApplicationStateType = "FAILING"
+ UnknownState ApplicationStateType = "UNKNOWN"
Review comment:
For the application state, currently, shim and core maintain different states. But moving on we should get them to be consistent. And the core should be the source of truth.
Please refer to https://github.com/apache/incubator-yunikorn-core/blob/f4ea4502a52e06e64e5abd0c47fedf2e08fe12e3/pkg/cache/application_state.go#L53-L61 to get the supported states by far.
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# generate the code with:
+# --output-base because this script should also be able to run inside the vendor dir of
+# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
+# instead of the $GOPATH directly. For normal projects this can be dropped.
+GO111MODULE=off
+bash k8s.io/code-generator/generate-groups.sh "all" \
+ github.com/apache/incubator-yunikorn-k8shim/pkg/client github.com/apache/incubator-yunikorn-k8shim/pkg/apis \
+ "yunikorn.apache.org:v1alpha1" \
+ #--output-base "$(dirname "${BASH_SOURCE[0]}")/../../.." \
+ #--go-header-file ./pkg/apis/apache.org/boilerplate.go.txt
Review comment:
is this the script to run the code gen? Can you pls add some documents about how to run this?
what's the reason to comment out these 2 lines? For the header file, if this is commented out, how the license headers were added to those generated files?
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter edited a comment on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bdacf71ac449ad54fe8e40ef7086b35e00af5c46&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
=======================================
Coverage 52.89% 52.89%
=======================================
Files 32 32
Lines 3025 3025
=======================================
Hits 1600 1600
Misses 1364 1364
Partials 61 61
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...e3141bd](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] yangwwei commented on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
yangwwei commented on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640350658
Can the code-gen be integrated into the Makefile?
I think we should try that, and add some doc directly in the Makefile if whoever wants to re-run the code gen.
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] HuangTing-Yao commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
HuangTing-Yao commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r440882451
##########
File path: Makefile
##########
@@ -165,6 +165,16 @@ push: image
docker push ${REGISTRY}/yunikorn:scheduler-${VERSION}
docker push ${REGISTRY}/yunikorn:admission-${VERSION}
+#Generate the CRD code with code-generator (release-1.14)
+
+# If you want to re-run the code-generator to generate code,
+# Please make sure the directory structure must be the example.
+# ex: github.com/apache/incubator-yunikorn-k8shim
Review comment:
If we change the structure of directory, it may affect the import path which in the generated file.
About the GOPATH, if GOPATH is empty, it will export GOPATH in generate-groups.sh, which set in sub-shell, won't bring back to parent shell.The GOPATH in environment should be set by user.
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] yangwwei commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r436827848
##########
File path: pkg/apis/yunikorn.apache.org/v1alpha1/type.go
##########
@@ -0,0 +1,72 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package v1alpha1
+
+import (
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+type Application struct {
+ metav1.TypeMeta `json:",inline"`
+ metav1.ObjectMeta `json:"metadata"`
+
+ Spec ApplicationSpec `json:"spec"`
+ Status ApplicationStatus `json:"status"`
+}
+
+//Spec part
+
+type ApplicationSpec struct {
+ MinMember int32 `json:"minMember"`
+ Queue string `json:"queue"`
+ MaxPendingSeconds int32 `json:"maxPendingSeconds,omitempty"`
+}
+
+//Status part
+
+type ApplicationStateType string
+
+const (
+ NewState ApplicationStateType = ""
+ SubmittedState ApplicationStateType = "SUBMITTED"
+ RunningState ApplicationStateType = "RUNNING"
+ CompletedState ApplicationStateType = "COMPLETED"
+ FailedState ApplicationStateType = "FAILED"
+ FailedSubmissionState ApplicationStateType = "SUBMISSION_FAILED"
+ PendingRerunState ApplicationStateType = "PENDING_RERUN"
+ InvalidatingState ApplicationStateType = "INVALIDATING"
+ SucceedingState ApplicationStateType = "SUCCEEDING"
+ FailingState ApplicationStateType = "FAILING"
+ UnknownState ApplicationStateType = "UNKNOWN"
Review comment:
I think we can try to move the definition to the scheduler-interface, then let this reuse that.
However, I am not sure if this is supported by the code-gen, this needs further investigation.
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# generate the code with:
+# --output-base because this script should also be able to run inside the vendor dir of
+# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
+# instead of the $GOPATH directly. For normal projects this can be dropped.
+GO111MODULE=off
+bash k8s.io/code-generator/generate-groups.sh "all" \
+ github.com/apache/incubator-yunikorn-k8shim/pkg/client github.com/apache/incubator-yunikorn-k8shim/pkg/apis \
+ "yunikorn.apache.org:v1alpha1" \
+ #--output-base "$(dirname "${BASH_SOURCE[0]}")/../../.." \
+ #--go-header-file ./pkg/apis/apache.org/boilerplate.go.txt
Review comment:
> About the license headers, code generator will read the boilerplate.go.txt which in k8s.io/code-generator/hack first. Base on my test, the --go-header-file won't work.
do you mean that specifying a customized file e.g /path/to/asf.header doesn't work with this command? Then how you add those license headers?
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# generate the code with:
+# --output-base because this script should also be able to run inside the vendor dir of
+# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
+# instead of the $GOPATH directly. For normal projects this can be dropped.
+GO111MODULE=off
Review comment:
We can turn this on for now. We can revisit this if we see some problems.
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# generate the code with:
+# --output-base because this script should also be able to run inside the vendor dir of
+# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
+# instead of the $GOPATH directly. For normal projects this can be dropped.
+GO111MODULE=off
+bash k8s.io/code-generator/generate-groups.sh "all" \
+ github.com/apache/incubator-yunikorn-k8shim/pkg/client github.com/apache/incubator-yunikorn-k8shim/pkg/apis \
+ "yunikorn.apache.org:v1alpha1" \
+ #--output-base "$(dirname "${BASH_SOURCE[0]}")/../../.." \
+ #--go-header-file ./pkg/apis/apache.org/boilerplate.go.txt
Review comment:
> About the license headers, code generator will read the boilerplate.go.txt which in k8s.io/code-generator/hack first. Base on my test, the --go-header-file won't work.
do you mean that specifying a customized file e.g /path/to/asf.header doesn't work with this command? Then how you add those license headers?
> The output-base is needed for the generators to output into the vendor dir instead of the $GOPATH directly. For normal projects this can be dropped." I don't think we need it.
Yes, I think we need it. We should support to manage everything outside of the $GOPATH dir
##########
File path: Makefile
##########
@@ -165,6 +165,12 @@ push: image
docker push ${REGISTRY}/yunikorn:scheduler-${VERSION}
docker push ${REGISTRY}/yunikorn:admission-${VERSION}
+#Generate the CRD code with code-generator (release-1.14)
+.PHONY: code_gen
+code_gen:
+ @echo "Generating CRD code"
+ bash ${GOPATH}/src/$(REPO)/apis/yunikorn.apache.org/update-codegen.sh
Review comment:
We are using `go mod` to manage dependencies. That doesn't require ${GOPATH} to be set.
Can we use the relative path to this file, `pkg/apis/yunikorn.apache.org/update-codegen.sh` to run this script?
In this case, I think we will need to specify the output dir, pls see the other comment I added in the `update-codegen.sh`
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,29 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# generate the code with:
+# --output-base because this script should also be able to run inside the vendor dir of
+# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
+# instead of the $GOPATH directly. For normal projects this can be dropped.
+
+bash ${GOPATH}/src/k8s.io/code-generator/generate-groups.sh "all" \
+ github.com/apache/incubator-yunikorn-k8shim/pkg/client github.com/apache/incubator-yunikorn-k8shim/pkg/apis \
Review comment:
I think we need to add `--output-base` param for this.
Please try to set up the repo outside of $GOPATH. As we are using `go mod` to manage dependencies, the repo can live outside the GOPATH.
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter edited a comment on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bdacf71ac449ad54fe8e40ef7086b35e00af5c46&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
=======================================
Coverage 52.89% 52.89%
=======================================
Files 32 32
Lines 3025 3025
=======================================
Hits 1600 1600
Misses 1364 1364
Partials 61 61
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...c4aa673](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter edited a comment on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bdacf71ac449ad54fe8e40ef7086b35e00af5c46&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
==========================================
- Coverage 52.92% 52.89% -0.04%
==========================================
Files 32 32
Lines 3025 3025
==========================================
- Hits 1601 1600 -1
- Misses 1363 1364 +1
Partials 61 61
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/shim/scheduler\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134/diff?src=pr&el=tree#diff-cGtnL3NoaW0vc2NoZWR1bGVyX21vY2suZ28=) | `85.00% <0.00%> (-0.84%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...b5554cc](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] HuangTing-Yao commented on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
HuangTing-Yao commented on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640463510
@sunilgovind I just refer from https://github.com/GoogleCloudPlatform/spark-on-k8s-operator/tree/master/pkg/apis/sparkoperator.k8s.io
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter edited a comment on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bdacf71ac449ad54fe8e40ef7086b35e00af5c46&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
==========================================
- Coverage 52.92% 52.89% -0.04%
==========================================
Files 32 32
Lines 3025 3025
==========================================
- Hits 1601 1600 -1
- Misses 1363 1364 +1
Partials 61 61
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/shim/scheduler\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134/diff?src=pr&el=tree#diff-cGtnL3NoaW0vc2NoZWR1bGVyX21vY2suZ28=) | `85.00% <0.00%> (-0.84%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...80cf203](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] kingamarton commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r436539941
##########
File path: pkg/apis/yunikorn.apache.org/v1alpha1/type.go
##########
@@ -0,0 +1,72 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package v1alpha1
+
+import (
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+type Application struct {
+ metav1.TypeMeta `json:",inline"`
+ metav1.ObjectMeta `json:"metadata"`
+
+ Spec ApplicationSpec `json:"spec"`
+ Status ApplicationStatus `json:"status"`
+}
+
+//Spec part
+
+type ApplicationSpec struct {
+ MinMember int32 `json:"minMember"`
+ Queue string `json:"queue"`
+ MaxPendingSeconds int32 `json:"maxPendingSeconds,omitempty"`
+}
+
+//Status part
+
+type ApplicationStateType string
+
+const (
+ NewState ApplicationStateType = ""
+ SubmittedState ApplicationStateType = "SUBMITTED"
+ RunningState ApplicationStateType = "RUNNING"
+ CompletedState ApplicationStateType = "COMPLETED"
+ FailedState ApplicationStateType = "FAILED"
+ FailedSubmissionState ApplicationStateType = "SUBMISSION_FAILED"
+ PendingRerunState ApplicationStateType = "PENDING_RERUN"
+ InvalidatingState ApplicationStateType = "INVALIDATING"
+ SucceedingState ApplicationStateType = "SUCCEEDING"
+ FailingState ApplicationStateType = "FAILING"
+ UnknownState ApplicationStateType = "UNKNOWN"
Review comment:
Since we will need to use the same states as in core, is it possible to define them somewhere in a common place and use that one both in core and here?
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] kingamarton commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r440721090
##########
File path: Makefile
##########
@@ -165,6 +165,16 @@ push: image
docker push ${REGISTRY}/yunikorn:scheduler-${VERSION}
docker push ${REGISTRY}/yunikorn:admission-${VERSION}
+#Generate the CRD code with code-generator (release-1.14)
+
+# If you want to re-run the code-generator to generate code,
+# Please make sure the directory structure must be the example.
+# ex: github.com/apache/incubator-yunikorn-k8shim
Review comment:
Is it possible to recognise where the repository is checked out instead of using a hardcoded directory structure? I tried to generate the code and in order to have it generated I have to change all my directory structure, what is a bit painfull.
It will also fail, if GOPATH is not set. It would be good to set it in the script, or at least document it that we need it set. For me a simple " export GOPATH="$HOME/go" solved the issue.
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter edited a comment on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bdacf71ac449ad54fe8e40ef7086b35e00af5c46&el=desc) will **increase** coverage by `0.14%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
==========================================
+ Coverage 52.92% 53.06% +0.14%
==========================================
Files 32 32
Lines 3025 3030 +5
==========================================
+ Hits 1601 1608 +7
+ Misses 1363 1359 -4
- Partials 61 63 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/cache/node\_coordinator.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL25vZGVfY29vcmRpbmF0b3IuZ28=) | `54.83% <0.00%> (-9.68%)` | :arrow_down: |
| [pkg/shim/scheduler\_mock.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134/diff?src=pr&el=tree#diff-cGtnL3NoaW0vc2NoZWR1bGVyX21vY2suZ28=) | `85.00% <0.00%> (-0.84%)` | :arrow_down: |
| [pkg/cache/nodes.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL25vZGVzLmdv) | `78.44% <0.00%> (+1.45%)` | :arrow_up: |
| [pkg/cache/external/scheduler\_cache.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2V4dGVybmFsL3NjaGVkdWxlcl9jYWNoZS5nbw==) | `34.21% <0.00%> (+4.95%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...4202f72](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] kingamarton commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
kingamarton commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r436590398
##########
File path: pkg/apis/yunikorn.apache.org/v1alpha1/type.go
##########
@@ -0,0 +1,71 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package v1alpha1
+
+import (
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// +genclient
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+type Application struct {
+ metav1.TypeMeta `json:",inline"`
+ metav1.ObjectMeta `json:"metadata"`
+
+ Spec ApplicationSpec `json:"spec"`
+ Status ApplicationStatus `json:"status"`
+}
+
+//Spec part
+
+type ApplicationSpec struct {
+ MinMember int32 `json:"minMember"`
+ Queue string `json:"queue"`
+ MaxPendingSeconds int32 `json:"maxPendingSeconds,omitempty"`
+}
+
+//Status part
+
+type ApplicationStateType string
+
+const (
+ NewApplicationState ApplicationStateType = "iota"
Review comment:
The string representation for New state should be "New". iota used here: https://github.com/apache/incubator-yunikorn-core/blob/f4ea4502a52e06e64e5abd0c47fedf2e08fe12e3/pkg/cache/application_state.go#L53-L61 is a keyword to represent successive integer constants
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter commented on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/192b962dd73a2a30511d2c924e1b8b946d07691d&el=desc) will **increase** coverage by `0.06%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
==========================================
+ Coverage 52.89% 52.95% +0.06%
==========================================
Files 32 32
Lines 3025 3025
==========================================
+ Hits 1600 1602 +2
+ Misses 1364 1362 -2
Partials 61 61
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/dispatcher/dispatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134/diff?src=pr&el=tree#diff-cGtnL2Rpc3BhdGNoZXIvZGlzcGF0Y2hlci5nbw==) | `87.15% <0.00%> (+1.83%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...e1a6b45](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter edited a comment on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bdacf71ac449ad54fe8e40ef7086b35e00af5c46&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
=======================================
Coverage 52.89% 52.89%
=======================================
Files 32 32
Lines 3025 3025
=======================================
Hits 1600 1600
Misses 1364 1364
Partials 61 61
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...c77df36](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] HuangTing-Yao commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
HuangTing-Yao commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r436472534
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# generate the code with:
+# --output-base because this script should also be able to run inside the vendor dir of
+# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
+# instead of the $GOPATH directly. For normal projects this can be dropped.
+GO111MODULE=off
+bash k8s.io/code-generator/generate-groups.sh "all" \
+ github.com/apache/incubator-yunikorn-k8shim/pkg/client github.com/apache/incubator-yunikorn-k8shim/pkg/apis \
+ "yunikorn.apache.org:v1alpha1" \
+ #--output-base "$(dirname "${BASH_SOURCE[0]}")/../../.." \
+ #--go-header-file ./pkg/apis/apache.org/boilerplate.go.txt
Review comment:
About the license headers, code generator will read the `boilerplate.go.txt` which in `k8s.io/code-generator/hack` first. Base on my test, the `--go-header-file` won't work.
The `--output-base` part, base on the description "because this script should also be able to run inside the vendor dir of k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir instead of the $GOPATH directly. For normal projects this can be dropped." I don't think we need it.
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] yangwwei merged pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
yangwwei merged pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter edited a comment on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bdacf71ac449ad54fe8e40ef7086b35e00af5c46&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
=======================================
Coverage 52.89% 52.89%
=======================================
Files 32 32
Lines 3025 3025
=======================================
Hits 1600 1600
Misses 1364 1364
Partials 61 61
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...deab5ff](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] HuangTing-Yao commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
HuangTing-Yao commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r438559057
##########
File path: Makefile
##########
@@ -165,6 +165,16 @@ push: image
docker push ${REGISTRY}/yunikorn:scheduler-${VERSION}
docker push ${REGISTRY}/yunikorn:admission-${VERSION}
+#Generate the CRD code with code-generator (release-1.14)
+
+# If you want to re-run the code-generator to generate code,
+# Please make sure the directory structure must be the example.
+# ex: github.com/apache/incubator-yunikorn-k8shim
+.PHONY: code_gen
+code_gen:
+ @echo "Generating CRD code"
+ bash pkg/apis/yunikorn.apache.org/update-codegen.sh
Review comment:
If we use`./` to run sh file, the `dirname` will fail, then the output-base setting will also fail.
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] HuangTing-Yao commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
HuangTing-Yao commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r436470335
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,31 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# generate the code with:
+# --output-base because this script should also be able to run inside the vendor dir of
+# k8s.io/kubernetes. The output-base is needed for the generators to output into the vendor dir
+# instead of the $GOPATH directly. For normal projects this can be dropped.
+GO111MODULE=off
Review comment:
According https://github.com/kubernetes/code-generator/issues/69
I turn GO111MODULE off.
But I think we can turn on the GO111MODULE
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] codecov-commenter edited a comment on pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#issuecomment-640351222
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=h1) Report
> Merging [#134](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bdacf71ac449ad54fe8e40ef7086b35e00af5c46&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #134 +/- ##
=======================================
Coverage 52.89% 52.89%
=======================================
Files 32 32
Lines 3025 3025
=======================================
Hits 1600 1600
Misses 1364 1364
Partials 61 61
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=footer). Last update [bdacf71...c4234b6](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/134?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] [incubator-yunikorn-k8shim] yangwwei commented on a change in pull request #134: [YUNIKORN-170] Create YuniKorn App CRD
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #134:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/134#discussion_r438512537
##########
File path: Makefile
##########
@@ -165,6 +165,16 @@ push: image
docker push ${REGISTRY}/yunikorn:scheduler-${VERSION}
docker push ${REGISTRY}/yunikorn:admission-${VERSION}
+#Generate the CRD code with code-generator (release-1.14)
+
+# If you want to re-run the code-generator to generate code,
+# Please make sure the directory structure must be the example.
+# ex: github.com/apache/incubator-yunikorn-k8shim
+.PHONY: code_gen
+code_gen:
+ @echo "Generating CRD code"
+ bash pkg/apis/yunikorn.apache.org/update-codegen.sh
Review comment:
NIT: do not use bash to run executable, some Linux box may not have bash installed, we do not need to depend on that. ./path/to/script.sh is enough.
Another thing is, can we move this script out of pkg folder? under pkg, that is expecting all .go src files. For script, I think we can create a top level folder: scripts
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,29 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# If you want to re-run the code-generator to generate code,
+# Please make sure the directory structure must be the example.
+# ex: github.com/apache/incubator-yunikorn-k8shim
+
+SCRIPT_ROOT=$(dirname ${BASH_SOURCE})
+
+bash pkg/apis/yunikorn.apache.org/generate-groups.sh "all" \
Review comment:
NIT: remove bash
##########
File path: pkg/apis/yunikorn.apache.org/custom-boilerplate.go.txt
##########
@@ -0,0 +1,17 @@
+/*
Review comment:
can we also move this file to project_root/scripts/
##########
File path: pkg/apis/yunikorn.apache.org/update-codegen.sh
##########
@@ -0,0 +1,29 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+#limitations under the License.
+#
+
+# If you want to re-run the code-generator to generate code,
+# Please make sure the directory structure must be the example.
+# ex: github.com/apache/incubator-yunikorn-k8shim
+
+SCRIPT_ROOT=$(dirname ${BASH_SOURCE})
+
+bash pkg/apis/yunikorn.apache.org/generate-groups.sh "all" \
+ github.com/apache/incubator-yunikorn-k8shim/pkg/client github.com/apache/incubator-yunikorn-k8shim/pkg/apis \
+ "yunikorn.apache.org:v1alpha1" \
+ --go-header-file "${SCRIPT_ROOT}"/custom-boilerplate.go.txt \
+ --output-base "$(dirname "${BASH_SOURCE[0]}")/../../../../../.."
Review comment:
suggest to move this script to project_root/scripts
----------------------------------------------------------------
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