You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "jiang1997 (via GitHub)" <gi...@apache.org> on 2023/03/17 15:46:07 UTC
[GitHub] [skywalking-python] jiang1997 opened a new pull request, #297: [wip]Build multi-architecture Docker images for Python agent
jiang1997 opened a new pull request, #297:
URL: https://github.com/apache/skywalking-python/pull/297
<!--
⚠️ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
- [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue url. Closes: <https://github.com/apache/skywalking/issues/10542>
- [ ] Update the [`CHANGELOG.md`](https://github.com/apache/skywalking-python/blob/master/CHANGELOG.md).
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#issuecomment-1475373558
For now, the version number has to be set inside [publish-docker.yaml](https://github.com/jiang1997/skywalking-python/blob/master/.github/workflows/publish-docker.yaml).
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1153432490
##########
docker/Makefile:
##########
@@ -31,24 +34,62 @@ py3.9-slim: BASE_PYTHON_IMAGE = python:3.9-slim
py3.10-slim: BASE_PYTHON_IMAGE = python:3.10-slim
py3.11-slim: BASE_PYTHON_IMAGE = python:3.11-slim
+push-py3.7: BASE_PYTHON_IMAGE = python:3.7
+push-py3.8: BASE_PYTHON_IMAGE = python:3.8
+push-py3.9: BASE_PYTHON_IMAGE = python:3.9
+push-py3.10: BASE_PYTHON_IMAGE = python:3.10
+push-py3.11: BASE_PYTHON_IMAGE = python:3.11
+push-py3.7-slim: BASE_PYTHON_IMAGE = python:3.7-slim
+push-py3.8-slim: BASE_PYTHON_IMAGE = python:3.8-slim
+push-py3.9-slim: BASE_PYTHON_IMAGE = python:3.9-slim
+push-py3.10-slim: BASE_PYTHON_IMAGE = python:3.10-slim
+push-py3.11-slim: BASE_PYTHON_IMAGE = python:3.11-slim
Review Comment:
It's okay to remove it. I'm reserving it for future needs.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on pull request #297: [wip]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#issuecomment-1475000277
> I can build locally when I set the `VERSION=1.0.0` (`make build-image VERSION=1.0.0`) so you might want to update your repo and set the same version.
>
> * We should also verify this in CI:
>
> * Build docker images without pushing to remote registry in PR checks, with `make build-image`.
> * Build and push images to ghcr.io with `make push-image`, [example](https://github.com/apache/skywalking-java/blob/main/.github/workflows/publish-docker.yaml)
When building a new docker image with CI, would it be better to use the local skywalking-python package instead of the one provided PyPI?
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #297: [wip]Build multi-architecture Docker images for Python agent
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#issuecomment-1474213322
I cant tell what's wrong from the traceback you posted though.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #297: [wip]Build multi-architecture Docker images for Python agent
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1140898600
##########
docker/Makefile:
##########
@@ -37,8 +37,10 @@ build: $(TARGETS)
push: $(PUSH_TARGETS)
$(TARGETS):
+docker buildx create --use --driver docker-container --name skywalking_python_main
Review Comment:
```suggestion
docker buildx create --use --driver docker-container --name skywalking_python_main
```
##########
docker/Makefile:
##########
@@ -37,8 +37,10 @@ build: $(TARGETS)
push: $(PUSH_TARGETS)
$(TARGETS):
+docker buildx create --use --driver docker-container --name skywalking_python_main
for p in $(P); do \
- $(D) build $(SW_BUILD_ARGS) \
+ $(D) buildx build $(SW_BUILD_ARGS) \
+ --platform linux/amd64,linux/arm64 \
Review Comment:
For `buildx`, we need `--load` or `--push` in the command.
> Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load.
So we have to modify the Make target a bit, an example is here https://github.com/apache/skywalking-java/blob/d357bdef7a3f1f9e78e6a8e29b372f3270e1acc2/Makefile#L45-L63
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1144400188
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,52 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ tag: ${{ github.sha }}
Review Comment:
> Ah, my mistake. I thought we had to adhere to the tag naming convention used by Docker Hub's.
> So, if I get this right, you mean to prefix the tag with the commit SHA, like `skywalking-python:e1a02359b239bd28de3f6d35fdc870250fa513d5-1.0.0-kafka-py3.10`?
Yeah but not quite, we don't need `1.0.0` here, just use `skywalking-python:e1a02359b239bd28de3f6d35fdc870250fa513d5-kafka-py3.10`, as the sha acts as a "version" number
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1144352708
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,52 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ tag: ${{ github.sha }}
Review Comment:
Ah, my mistake. I thought we had to adhere to the tag naming convention used by Docker Hub's.
So, if I get this right, you mean to prefix the tag with the commit SHA, like `skywalking-python:e1a02359b239bd28de3f6d35fdc870250fa513d5-1.0.0-kafka-py3.10`?
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1147600939
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,51 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ VERSION: ${{ github.sha }}
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Log in to the Container registry
+ uses: docker/login-action@v1.10.0
+ with:
+ registry: ${{ env.HUB }}
+ username: ${{ github.actor }}
+ password: ${{ secrets.GITHUB_TOKEN }}
+ - name: Build and push docker image
+ run: |
+ make push-image
Review Comment:
I have tried `make -j 10 push-image`, but seems that `docker buildx` can't run parallel.
Firstly, container `skywalking_python_main` can't be shared between parallel building tasks, secondly, even though we create target-specific containers, because [this line](https://github.com/jiang1997/skywalking-python/blob/a2584055130f5b8eb2f0c474bbac8213eef2094c/docker/Makefile#L81) works at system level, CI still runs into errors.
After some digging, I found that the most time-consuming step is building gRPC related whl packages when building image for arm64 `python3.11`. Cuz there is no binary whl package off the shelf so far. https://github.com/grpc/grpc/issues/32454
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1141700680
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,53 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ VERSION: 1.0.0
Review Comment:
As this workflow push the snapshot docker images, we should use the github commit sha (`${{ github.sha }}`) as the image tag, and ensure the images pushed are with latest codes in main branch.
##########
docker/Dockerfile:
##########
@@ -21,7 +21,9 @@ FROM ${BASE_PYTHON_IMAGE}
ARG SW_PYTHON_AGENT_PROTOCOL
ARG SW_PYTHON_AGENT_VERSION
-RUN pip install --no-cache-dir "apache-skywalking[${SW_PYTHON_AGENT_PROTOCOL}]==${SW_PYTHON_AGENT_VERSION}"
+COPY dist/apache_skywalking*.whl /tmp/
Review Comment:
Is the `.whl` file is architecture independent? If it's architecture dependent we cannot simply copy the `.whl` file into the Docker image, instead we should build the `.whl` file inside Dockerfile. @Superskyyy any input?
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1141749629
##########
docker/Dockerfile:
##########
@@ -21,7 +21,9 @@ FROM ${BASE_PYTHON_IMAGE}
ARG SW_PYTHON_AGENT_PROTOCOL
ARG SW_PYTHON_AGENT_VERSION
-RUN pip install --no-cache-dir "apache-skywalking[${SW_PYTHON_AGENT_PROTOCOL}]==${SW_PYTHON_AGENT_VERSION}"
+COPY dist/apache_skywalking*.whl /tmp/
Review Comment:
> Is the `.whl` file is architecture independent? If it's architecture dependent we cannot simply copy the `.whl` file into the Docker image, instead we should build the `.whl` file inside Dockerfile. @Superskyyy any input?
Our current `.whl` file is platform independent (`none`) and should be safe to copy (our codebase is currently in pure python), but let's not rely on this. I think we should go for the second option and build wheels inside dockerfile.
(Btw if you try the python agent wheel on mac m1, it works right?)
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on pull request #297: [wip]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#issuecomment-1474046122
I have simply tried `docker build buildx` by referring to https://github.com/apache/skywalking-eyes/pull/132 .
But it turns out that `skywalking-python` can't be installed directly on linux/arm64.
```
=> => extracting sha256:a2ae4d14db5909da83c9bbe5296aa28df736473d058728285f074eda6e427b0e 0
=> [linux/amd64 2/2] RUN pip install --no-cache-dir "apache-skywalking[grpc]==0.8.0" 26
=> ERROR [linux/arm64 2/2] RUN pip install --no-cache-dir "apache-skywalking[grpc]==0.8.0" 10
------
> [linux/arm64 2/2] RUN pip install --no-cache-dir "apache-skywalking[grpc]==0.8.0":
#0 8.577 ERROR: Exception:
#0 8.577 Traceback (most recent call last):
#0 8.577 File "/usr/local/lib/python3.8/site-packages/pip/_internal/cli/base_command.py", line 167, in exc_logging_wrapper
#0 8.577 status = run_func(*args)
#0 8.577 File "/usr/local/lib/python3.8/site-packages/pip/_internal/cli/req_command.py", line 205, in wrapper
#0 8.577 return func(self, options, args)
#0 8.577 File "/usr/local/lib/python3.8/site-packages/pip/_internal/commands/install.py", line 285, in run
#0 8.577 session = self.get_default_session(options)
#0 8.577 File "/usr/local/lib/python3.8/site-packages/pip/_internal/cli/req_command.py", line 75, in get_default_session
```
To delve deeper, I'll be creating a Linux/ARM64 environment.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1148503536
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,51 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ VERSION: ${{ github.sha }}
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Log in to the Container registry
+ uses: docker/login-action@v1.10.0
+ with:
+ registry: ${{ env.HUB }}
+ username: ${{ github.actor }}
+ password: ${{ secrets.GITHUB_TOKEN }}
+ - name: Build and push docker image
+ run: |
+ make push-image
Review Comment:
I discovered that with the [`--builder`](https://docs.docker.com/engine/reference/commandline/buildx/#builder) option in Docker, we can make multiple targets simultaneously, but not to much(with `make -j 10` may run out of disk space).
Compared with no-parallel building, we can save about 10 minutes.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy merged pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy merged PR #297:
URL: https://github.com/apache/skywalking-python/pull/297
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #297: [wip]Build multi-architecture Docker images for Python agent
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#issuecomment-1474204828
Please remember to add a changelog.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1143499377
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,52 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ tag: ${{ github.sha }}
+ VERSION: 1.0.0
Review Comment:
... but it's `VERSION` here,
##########
docker/Makefile:
##########
@@ -31,24 +34,49 @@ py3.9-slim: BASE_PYTHON_IMAGE = python:3.9-slim
py3.10-slim: BASE_PYTHON_IMAGE = python:3.10-slim
py3.11-slim: BASE_PYTHON_IMAGE = python:3.11-slim
+push-py3.7: BASE_PYTHON_IMAGE = python:3.7
+push-py3.8: BASE_PYTHON_IMAGE = python:3.8
+push-py3.9: BASE_PYTHON_IMAGE = python:3.9
+push-py3.10: BASE_PYTHON_IMAGE = python:3.10
+push-py3.11: BASE_PYTHON_IMAGE = python:3.11
+push-py3.7-slim: BASE_PYTHON_IMAGE = python:3.7-slim
+push-py3.8-slim: BASE_PYTHON_IMAGE = python:3.8-slim
+push-py3.9-slim: BASE_PYTHON_IMAGE = python:3.9-slim
+push-py3.10-slim: BASE_PYTHON_IMAGE = python:3.10-slim
+push-py3.11-slim: BASE_PYTHON_IMAGE = python:3.11-slim
+
PUSH_TARGETS := $(TARGETS:%=push-%)
build: $(TARGETS)
push: $(PUSH_TARGETS)
+
+
$(TARGETS):
+ cd ..; \
for p in $(P); do \
$(D) build $(SW_BUILD_ARGS) \
+ -f docker/Dockerfile \
--build-arg BASE_PYTHON_IMAGE=$(BASE_PYTHON_IMAGE) \
--build-arg SW_PYTHON_AGENT_PROTOCOL=$$p \
--build-arg SW_PYTHON_AGENT_VERSION=${AGENT_VERSION} \
- -t apache/skywalking-python:${AGENT_VERSION}-$$p-$@ \
+ -t $(HUB)/$(PROJECT):${AGENT_VERSION}-$$p-$@ \
Review Comment:
We are using `AGENT_VERSION` as the tag here,
##########
docker/Dockerfile:
##########
@@ -16,12 +16,13 @@
ARG BASE_PYTHON_IMAGE
-FROM ${BASE_PYTHON_IMAGE}
-
-ARG SW_PYTHON_AGENT_PROTOCOL
-ARG SW_PYTHON_AGENT_VERSION
-
-RUN pip install --no-cache-dir "apache-skywalking[${SW_PYTHON_AGENT_PROTOCOL}]==${SW_PYTHON_AGENT_VERSION}"
+FROM ${BASE_PYTHON_IMAGE} as builder
+RUN pip install poetry
+COPY ./ /tmp/
+RUN make -C /tmp/ package
+FROM ${BASE_PYTHON_IMAGE} as final
+COPY --from=builder /tmp/dist/apache_skywalking*.whl /tmp/
Review Comment:
Let's remove the .whl file after installation to reduce the image size
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,52 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ tag: ${{ github.sha }}
Review Comment:
... and this is not 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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1148503536
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,51 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ VERSION: ${{ github.sha }}
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Log in to the Container registry
+ uses: docker/login-action@v1.10.0
+ with:
+ registry: ${{ env.HUB }}
+ username: ${{ github.actor }}
+ password: ${{ secrets.GITHUB_TOKEN }}
+ - name: Build and push docker image
+ run: |
+ make push-image
Review Comment:
I discovered that with the [`--builder`](https://docs.docker.com/engine/reference/commandline/buildx/#builder) option in Docker, we can make multiple targets simultaneously, but not to much(with `make -j 10` may run out of disk space).
Compared with non-parallel building, we can save about 10 minutes.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1144360132
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,52 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ tag: ${{ github.sha }}
+ VERSION: 1.0.0
Review Comment:
Actually, the following Makefile will pass the value of `VERSION` to `AGENT_VERSION`.
https://github.com/apache/skywalking-python/blob/46a0f8b8501901e8fca89b4f115094a1f4a363d1/Makefile#L118-L119
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1147600939
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,51 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ VERSION: ${{ github.sha }}
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Log in to the Container registry
+ uses: docker/login-action@v1.10.0
+ with:
+ registry: ${{ env.HUB }}
+ username: ${{ github.actor }}
+ password: ${{ secrets.GITHUB_TOKEN }}
+ - name: Build and push docker image
+ run: |
+ make push-image
Review Comment:
I have tried `make -j 10 push-image`, but seems that `docker buildx` can't run parallel.
Firstly, container `skywalking_python_main` can't be shared between parallel building tasks, secondly, even though we create target-specific containers, because [this line](https://github.com/jiang1997/skywalking-python/blob/a2584055130f5b8eb2f0c474bbac8213eef2094c/docker/Makefile#L81) works at system level, CI still runs into errors.
After some digging, I found that the most time-consuming step is building gRPC related whl packages when building image for arm64 `python3.11`. Cuz there is no needed binary whl package off the shelf so far. https://github.com/grpc/grpc/issues/32454
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #297: [wip]Build multi-architecture Docker images for Python agent
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1140899724
##########
docker/Makefile:
##########
@@ -37,8 +37,10 @@ build: $(TARGETS)
push: $(PUSH_TARGETS)
$(TARGETS):
+docker buildx create --use --driver docker-container --name skywalking_python_main
Review Comment:
We need to remove the buildx (`docker buildx rm skywalking_python_main`) at the end of this target.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1151731376
##########
docker/Makefile:
##########
@@ -31,24 +34,62 @@ py3.9-slim: BASE_PYTHON_IMAGE = python:3.9-slim
py3.10-slim: BASE_PYTHON_IMAGE = python:3.10-slim
py3.11-slim: BASE_PYTHON_IMAGE = python:3.11-slim
+push-py3.7: BASE_PYTHON_IMAGE = python:3.7
+push-py3.8: BASE_PYTHON_IMAGE = python:3.8
+push-py3.9: BASE_PYTHON_IMAGE = python:3.9
+push-py3.10: BASE_PYTHON_IMAGE = python:3.10
+push-py3.11: BASE_PYTHON_IMAGE = python:3.11
+push-py3.7-slim: BASE_PYTHON_IMAGE = python:3.7-slim
+push-py3.8-slim: BASE_PYTHON_IMAGE = python:3.8-slim
+push-py3.9-slim: BASE_PYTHON_IMAGE = python:3.9-slim
+push-py3.10-slim: BASE_PYTHON_IMAGE = python:3.10-slim
+push-py3.11-slim: BASE_PYTHON_IMAGE = python:3.11-slim
Review Comment:
Do we need to remove this ? You removed 3.11 in the `TARGETS`
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1144352708
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,52 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ tag: ${{ github.sha }}
Review Comment:
Ah, my mistake. I thought we had to adhere to the tag naming convention used by Docker Hub's.
So, if I get this right, you mean to prefix the tag with the commit SHA, like skywalking-python:e1a02359b239bd28de3f6d35fdc870250fa513d5-1.0.0-kafka-py3.10?
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #297: [feat]Build multi-architecture Docker images for Python agent
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#discussion_r1145657652
##########
.github/workflows/publish-docker.yaml:
##########
@@ -0,0 +1,51 @@
+# 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.
+
+name: publish-docker
+
+on:
+ push:
+ branches:
+ - master
+
+env:
+ HUB: ghcr.io/apache/skywalking-python
+ PROJECT: skywalking-python
+
+jobs:
+ build-docker:
+ if: github.repository == 'apache/skywalking-python'
+ runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ packages: write
+ timeout-minutes: 30
+ env:
+ VERSION: ${{ github.sha }}
+ steps:
+ - uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Log in to the Container registry
+ uses: docker/login-action@v1.10.0
+ with:
+ registry: ${{ env.HUB }}
+ username: ${{ github.actor }}
+ password: ${{ secrets.GITHUB_TOKEN }}
+ - name: Build and push docker image
+ run: |
+ make push-image
Review Comment:
I tried to build and push locally and hours passed it is still running, I think we can parallize this by adding `make -j 10 push-image`?
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on pull request #297: [wip]Build multi-architecture Docker images for Python agent
Posted by "jiang1997 (via GitHub)" <gi...@apache.org>.
jiang1997 commented on PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#issuecomment-1474994871
> I have simply tried `docker build buildx` by referring to [apache/skywalking-eyes#132](https://github.com/apache/skywalking-eyes/pull/132) . But it turns out that `skywalking-python` can't be installed directly on linux/arm64.
>
> ```
> => => extracting sha256:a2ae4d14db5909da83c9bbe5296aa28df736473d058728285f074eda6e427b0e 0
> => [linux/amd64 2/2] RUN pip install --no-cache-dir "apache-skywalking[grpc]==0.8.0" 26
> => ERROR [linux/arm64 2/2] RUN pip install --no-cache-dir "apache-skywalking[grpc]==0.8.0" 10
> ------
> > [linux/arm64 2/2] RUN pip install --no-cache-dir "apache-skywalking[grpc]==0.8.0":
> #0 8.577 ERROR: Exception:
> #0 8.577 Traceback (most recent call last):
> #0 8.577 File "/usr/local/lib/python3.8/site-packages/pip/_internal/cli/base_command.py", line 167, in exc_logging_wrapper
> #0 8.577 status = run_func(*args)
> #0 8.577 File "/usr/local/lib/python3.8/site-packages/pip/_internal/cli/req_command.py", line 205, in wrapper
> #0 8.577 return func(self, options, args)
> #0 8.577 File "/usr/local/lib/python3.8/site-packages/pip/_internal/commands/install.py", line 285, in run
> #0 8.577 session = self.get_default_session(options)
> #0 8.577 File "/usr/local/lib/python3.8/site-packages/pip/_internal/cli/req_command.py", line 75, in get_default_session
> ```
>
> To delve deeper, I'll be creating a Linux/ARM64 environment.
solved according to this https://github.com/docker/buildx/issues/464#issuecomment-740434862
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on pull request #297: [wip]Build multi-architecture Docker images for Python agent
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on PR #297:
URL: https://github.com/apache/skywalking-python/pull/297#issuecomment-1475042913
>
>
>
>
> > I can build locally when I set the `VERSION=1.0.0` (`make build-image VERSION=1.0.0`) so you might want to update your repo and set the same version.
>
> >
>
> > * We should also verify this in CI:
>
> >
>
> > * Build docker images without pushing to remote registry in PR checks, with `make build-image`.
>
> > * Build and push images to ghcr.io with `make push-image`, [example](https://github.com/apache/skywalking-java/blob/main/.github/workflows/publish-docker.yaml)
>
>
>
> When building a new docker image with CI, would it be better to use the local skywalking-python package instead of the one provided by PyPI?
>
>
Yes sure we should use the local skywalking Python package because that's what we want to verify.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org