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 2022/11/21 20:43:24 UTC
[GitHub] [yunikorn-k8shim] kannon92 opened a new pull request, #487: [YUNIKORN-1299] amd64 tag and details for building locally
kannon92 opened a new pull request, #487:
URL: https://github.com/apache/yunikorn-k8shim/pull/487
### What is this PR for?
While writing a developer document about Kind, I discovered a few issues with building the shim-repo. The build adds the architect to the docker tag but none of the docs reflect this.
### What type of PR is it?
* [ ] - Bug Fix
* [ ] - Improvement
* [ ] - Feature
* [ x ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
* Open an issue on Jira https://issues.apache.org/jira/browse/YUNIKORN/
* Put link here, and add [YUNIKORN-*Jira number*] in PR title, eg. `[YUNIKORN-2] Gang scheduling interface parameters`
[YUNIKORN-1299](https://issues.apache.org/jira/browse/YUNIKORN-1299)
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] codecov[bot] commented on pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #487:
URL: https://github.com/apache/yunikorn-k8shim/pull/487#issuecomment-1322630361
# [Codecov](https://codecov.io/gh/apache/yunikorn-k8shim/pull/487?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 [#487](https://codecov.io/gh/apache/yunikorn-k8shim/pull/487?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7169a1c) into [master](https://codecov.io/gh/apache/yunikorn-k8shim/commit/3f90831f0e70aaf0e24ee956a3329363717c9a1d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3f90831) will **not change** coverage.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #487 +/- ##
=======================================
Coverage 66.98% 66.98%
=======================================
Files 42 42
Lines 7300 7300
=======================================
Hits 4890 4890
Misses 2234 2234
Partials 176 176
```
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] kannon92 commented on a diff in pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
Posted by GitBox <gi...@apache.org>.
kannon92 commented on code in PR #487:
URL: https://github.com/apache/yunikorn-k8shim/pull/487#discussion_r1029758018
##########
README.md:
##########
@@ -46,6 +46,12 @@ Run `./k8s_yunikorn_scheduler -help` to see all options.
In case you get an error relating to `checksum mismatch`, run `go clean -modcache` and then rerun `make build`.
### Build run
+
Review Comment:
Sorry about that. I remember seeing a warning about this and I thought it was required. But I tested again and it isn't.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] kannon92 commented on a diff in pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
Posted by GitBox <gi...@apache.org>.
kannon92 commented on code in PR #487:
URL: https://github.com/apache/yunikorn-k8shim/pull/487#discussion_r1029797164
##########
README.md:
##########
@@ -71,12 +77,12 @@ You can set `DOCKER_ARCH`, `REGISTRY` and `VERSION` in the commandline to build
```
make image DOCKER_ARCH=amd64 REGISTRY=yunikorn VERSION=latest
```
-This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
+This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-amd64-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
You can run following command to retrieve the meta info for a docker image build, such as component revisions, date of the build, etc.
```
-docker inspect --format='{{.Config.Labels}}' yunikorn/yunikorn:scheduler-latest
+docker inspect --format='{{.Config.Labels}}' yunikorn/yunikorn:scheduler-amd64-latest
Review Comment:
If its fine, I think we can leave it as is. I added some text to the Makefile to reflect where that tag comes from. So I think its good 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.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] craigcondit closed pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
Posted by GitBox <gi...@apache.org>.
craigcondit closed pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
URL: https://github.com/apache/yunikorn-k8shim/pull/487
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] kannon92 commented on a diff in pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
Posted by GitBox <gi...@apache.org>.
kannon92 commented on code in PR #487:
URL: https://github.com/apache/yunikorn-k8shim/pull/487#discussion_r1029756555
##########
README.md:
##########
@@ -71,12 +77,12 @@ You can set `DOCKER_ARCH`, `REGISTRY` and `VERSION` in the commandline to build
```
make image DOCKER_ARCH=amd64 REGISTRY=yunikorn VERSION=latest
```
-This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
+This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-amd64-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
You can run following command to retrieve the meta info for a docker image build, such as component revisions, date of the build, etc.
```
-docker inspect --format='{{.Config.Labels}}' yunikorn/yunikorn:scheduler-latest
+docker inspect --format='{{.Config.Labels}}' yunikorn/yunikorn:scheduler-amd64-latest
Review Comment:
This may be a controversial change since you may have developers building accross many different repos. Not exactly sure how to make this better.
I could set it to the most common OS for your developers and anyone else can modify this locally. Open to suggestions on these changes?
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] craigcondit commented on a diff in pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
Posted by GitBox <gi...@apache.org>.
craigcondit commented on code in PR #487:
URL: https://github.com/apache/yunikorn-k8shim/pull/487#discussion_r1028742102
##########
README.md:
##########
@@ -71,12 +77,12 @@ You can set `DOCKER_ARCH`, `REGISTRY` and `VERSION` in the commandline to build
```
make image DOCKER_ARCH=amd64 REGISTRY=yunikorn VERSION=latest
```
-This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
+This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-amd64-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
Review Comment:
Can we add some language reflecting that this is a architecture-specific tag? It will be different for arm, etc.
##########
README.md:
##########
@@ -46,6 +46,12 @@ Run `./k8s_yunikorn_scheduler -help` to see all options.
In case you get an error relating to `checksum mismatch`, run `go clean -modcache` and then rerun `make build`.
### Build run
+
Review Comment:
This is not a requirement to run YuniKorn. We should not put this section in.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] craigcondit commented on a diff in pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
Posted by GitBox <gi...@apache.org>.
craigcondit commented on code in PR #487:
URL: https://github.com/apache/yunikorn-k8shim/pull/487#discussion_r1029764229
##########
README.md:
##########
@@ -71,12 +77,12 @@ You can set `DOCKER_ARCH`, `REGISTRY` and `VERSION` in the commandline to build
```
make image DOCKER_ARCH=amd64 REGISTRY=yunikorn VERSION=latest
```
-This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
+This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-amd64-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
You can run following command to retrieve the meta info for a docker image build, such as component revisions, date of the build, etc.
```
-docker inspect --format='{{.Config.Labels}}' yunikorn/yunikorn:scheduler-latest
+docker inspect --format='{{.Config.Labels}}' yunikorn/yunikorn:scheduler-amd64-latest
Review Comment:
I think what you have is fine. Maybe we could just put something at the top that wherever "amd64" is present in the examples, it should be replaced with the architecture you are building for.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] kannon92 commented on a diff in pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
Posted by GitBox <gi...@apache.org>.
kannon92 commented on code in PR #487:
URL: https://github.com/apache/yunikorn-k8shim/pull/487#discussion_r1032614865
##########
README.md:
##########
@@ -71,12 +77,12 @@ You can set `DOCKER_ARCH`, `REGISTRY` and `VERSION` in the commandline to build
```
make image DOCKER_ARCH=amd64 REGISTRY=yunikorn VERSION=latest
```
-This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
+This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-amd64-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
Review Comment:
Done.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] kannon92 commented on a diff in pull request #487: [YUNIKORN-1299] amd64 tag and details for building locally
Posted by GitBox <gi...@apache.org>.
kannon92 commented on code in PR #487:
URL: https://github.com/apache/yunikorn-k8shim/pull/487#discussion_r1034805431
##########
README.md:
##########
@@ -71,12 +77,12 @@ You can set `DOCKER_ARCH`, `REGISTRY` and `VERSION` in the commandline to build
```
make image DOCKER_ARCH=amd64 REGISTRY=yunikorn VERSION=latest
```
-This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
+This command will build an amd64 binary executable with version `latest` and the docker image tag is `yunikorn/yunikorn:scheduler-amd64-latest`. If not specified, `DOCKER_ARCH` defaults to the build host's architecture.
You can run following command to retrieve the meta info for a docker image build, such as component revisions, date of the build, etc.
```
-docker inspect --format='{{.Config.Labels}}' yunikorn/yunikorn:scheduler-latest
+docker inspect --format='{{.Config.Labels}}' yunikorn/yunikorn:scheduler-amd64-latest
Review Comment:
I think this is ready for review whenever you can get a chance.
--
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: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org