You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2022/07/07 03:18:00 UTC
[GitHub] [submarine] ianshen1104 opened a new pull request, #976: SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3
ianshen1104 opened a new pull request, #976:
URL: https://github.com/apache/submarine/pull/976
### What is this PR for?
<!-- A few sentences describing the overall goals of the pull request's commits.
First time? Check out the contributing guide - https://submarine.apache.org/contribution/contributions.html
-->
Add app.kubernetes.io labels recommended here by Kubernetes
https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/
### What type of PR is it?
Improvement
### Todos
* [x] - Add app.kubernetes.io labels in submarine-cloud-v3 artifacts
### What is the Jira issue?
<!-- * Open an issue on Jira https://issues.apache.org/jira/browse/SUBMARINE/
* Put link here, and add [SUBMARINE-*Jira number*] in PR title, eg. `SUBMARINE-23. PR title`
-->
https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-1290
### How should this be tested?
<!--
* First time? Setup Travis CI as described on https://submarine.apache.org/contribution/contributions.html#continuous-integration
* Strongly recommended: add automated unit tests for any new or changed behavior
* Outline any manual steps to test the PR here.
-->
Follow the `submarine-cloud-v3/docs/developer-guide.md`
### Screenshots (if appropriate)
### Questions:
* Do the license files need updating? No
* Are there breaking changes for older versions? No
* Does this need new documentation? No
--
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.
To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] MortalHappiness commented on a diff in pull request #976: SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3
Posted by GitBox <gi...@apache.org>.
MortalHappiness commented on code in PR #976:
URL: https://github.com/apache/submarine/pull/976#discussion_r921182913
##########
submarine-cloud-v3/artifacts/submarine-database.yaml:
##########
@@ -20,6 +20,10 @@ apiVersion: v1
kind: PersistentVolumeClaim
metadata:
name: submarine-database-pvc
+ labels:
+ app.kubernetes.io/name: submarine-database
+ app.kubernetes.io/version: "0.8.0-SNAPSHOT"
+ app.kubernetes.io/component: database
spec:
Review Comment:
I think we can write the version information in golang file instead of yaml file. For example, for this PersistentVolumeClaim, see the golang function below. We can set `pvc.Labels` before we create the resource. This version information can be written as golang constant.
https://github.com/apache/submarine/blob/e7ed4a5b182a7cc3cad32f406a8327e6c788d966/submarine-cloud-v3/controllers/submarine_database.go#L35-L46
For your reference:
+ https://pkg.go.dev/k8s.io/api/core/v1#PersistentVolumeClaim
+ https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#ObjectMeta
--
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.
To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] codecov[bot] commented on pull request #976: SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #976:
URL: https://github.com/apache/submarine/pull/976#issuecomment-1179482818
# [Codecov](https://codecov.io/gh/apache/submarine/pull/976?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#976](https://codecov.io/gh/apache/submarine/pull/976?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1970c71) into [master](https://codecov.io/gh/apache/submarine/commit/e7ed4a5b182a7cc3cad32f406a8327e6c788d966?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e7ed4a5) will **not change** coverage.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #976 +/- ##
========================================
Coverage 9.18% 9.18%
Complexity 670 670
========================================
Files 234 234
Lines 23716 23716
Branches 3475 3475
========================================
Hits 2178 2178
Misses 21407 21407
Partials 131 131
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/submarine/pull/976?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/submarine/pull/976?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e7ed4a5...1970c71](https://codecov.io/gh/apache/submarine/pull/976?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] ianshen1104 commented on a diff in pull request #976: SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3
Posted by GitBox <gi...@apache.org>.
ianshen1104 commented on code in PR #976:
URL: https://github.com/apache/submarine/pull/976#discussion_r935977079
##########
submarine-cloud-v3/artifacts/submarine-database.yaml:
##########
@@ -20,6 +20,10 @@ apiVersion: v1
kind: PersistentVolumeClaim
metadata:
name: submarine-database-pvc
+ labels:
+ app.kubernetes.io/name: submarine-database
+ app.kubernetes.io/version: "0.8.0-SNAPSHOT"
+ app.kubernetes.io/component: database
spec:
Review Comment:
Thank you for your suggestion @MortalHappiness,
I have changed the code to perform label setting in golang functions instead of yaml 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.
To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] cdmikechen commented on pull request #976: SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3
Posted by GitBox <gi...@apache.org>.
cdmikechen commented on PR #976:
URL: https://github.com/apache/submarine/pull/976#issuecomment-1178431844
Can we replace `app.kubernetes.io/version` value using a consistent method? I personally don't think hard-coding the version is a good way.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] xunliu commented on a diff in pull request #976: SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3
Posted by GitBox <gi...@apache.org>.
xunliu commented on code in PR #976:
URL: https://github.com/apache/submarine/pull/976#discussion_r963254257
##########
submarine-cloud-v3/config/rbac/role.yaml:
##########
@@ -1,20 +1,3 @@
-#
-# 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.
-#
Review Comment:
The yaml file also needs to retain the `Apache license` description.
##########
submarine-cloud-v3/config/crd/bases/submarine.apache.org_submarines.yaml:
##########
@@ -1,20 +1,3 @@
-#
-# 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.
-#
-
Review Comment:
hi @ianshen1104
The yaml file also needs to retain the `Apache license` description.
What is the reason for deletion?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] ianshen1104 commented on pull request #976: SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3
Posted by GitBox <gi...@apache.org>.
ianshen1104 commented on PR #976:
URL: https://github.com/apache/submarine/pull/976#issuecomment-1178468952
@cdmikechen thanks for your feedback!
I will update my changes soon and push again.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org