Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support publish_server_image to dockerhub automatically (V2) #1

Merged
merged 15 commits into from
Sep 15, 2023

Conversation

aroundabout
Copy link
Collaborator

@aroundabout aroundabout commented Aug 8, 2023

Purpose of PR

subtask of apache/incubator-hugegraph#840

  • keep the tags same with server (like hugegraph/hugegraph:1.0.0)

Main change

Api reference: This api help us to update the github repo var Update a repository variable

old desgin doc: apache/incubator-hugegraph#2254
new design doc: extra design doc

new change

  1. In publish release version, we use one input with regex instead of the two input "tag" and release

Before Merge

  1. Create a branch in apache/hugegraph repo, then I can submit a pr of the release-1.0.0 with correct dockerfile and other shell. it needs the admin to create.

  2. Before merge, admin should confirm that there are the repository secrets "DOCKERHUB_PASSWORD", "DOCKERHUB_USERNAME", "PAT" are provided, where "PAT" is a token accessible to the repo.

Repository variables "LATEST_SHA" should be provided. Because there is no "create an update" api, "LATEST_SHA" are necessary.

@CLAassistant
Copy link

CLAassistant commented Aug 8, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@imbajin imbajin changed the title Create publish_server_image.yml feat: support publish_server_image to dockerhub automatically (V2) Aug 16, 2023
@imbajin imbajin requested a review from coderzc August 16, 2023 18:20
.github/workflows/pushlish_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/pushlish_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/pushlish_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release_server_image.yml Outdated Show resolved Hide resolved
@imbajin
Copy link
Collaborator

imbajin commented Aug 26, 2023

@Radeity could also take a look for it when free, THX

.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved

- name: Get current commit-hash
run: |
current_commit_hash=$(git rev-parse HEAD)
Copy link
Collaborator

@imbajin imbajin Aug 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: seems current_commit VS last_commit is enough

rename to current_commit (remove hash suffix)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to keep the style same as the one in name "Get current commit-hash"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to keep the style same as the one in name "Get current commit-hash"

fine, u could update the title style or not

.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_release_server_image.yml Outdated Show resolved Hide resolved
Comment on lines +59 to +60
cache-from: type=gha
cache-to: type=gha,mode=max
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: check if the docker image cache work well (seems doesn't take effect now?)

could fix it in next/another PR, just address it

Comment on lines 11 to 14
env:
repository_url: apache/hugegraph
branch: master
image_url: hugegraph/hugegraph:latest
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refer server CI file, maybe we could:

  1. use the env.xx directly? like
image 2. put all params here (to keep the logic clean) image

and search/ask GPT for the difference between $param & ${{env.param}} in action (seems $param looks better)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The difference is mentioned in https://hugegraph.feishu.cn/docx/U8LKdSsxWoyxlnxKSXPcNoOanJd#HpV9dR7UfoCbyOxsxwNcRucHnCf
    image
  2. I do not find a way to use regexp when setting the env, same with gpt
    image

Comment on lines +33 to +34
repository: ${{ env.REPOSITORY_URL }}
ref: ${{ env.BRANCH }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the difference between ${{ env.BRANCH }} & $BRANCH? how to choose

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do not have real difference, but in Github action docs, it said that:
"Runner environment variables are always interpolated on the runner machine. However, parts of a workflow are processed by GitHub Actions and are not sent to the runner. You cannot use environment variables in these parts of a workflow file. Instead, you can use contexts. For example, an if conditional, which determines whether a job or step is sent to the runner, is always processed by GitHub Actions. You can use a context in an if conditional statement to access the value of an variable."
It mease that we can not directly use $BRANCH in these parts like "if" or "with", and we must use env context to get them. Because they are processed by github action, but not runner machine. You can not use $BRANCH to get the value. In github action, there is not $BRANCH, but only the env context or other context.
But in "run statement", these vars are sent to runner machine instead of github action, where we can use $BRANCH directly to get the value.

BTW, in "if conditional statement", use ${{ }} seems to be the recommended way, I have fixed it.

-d '{"name":"'"$var_name"'","value":"'"$current_commit_hash"'"}'
-H "Authorization: Bearer $GITHUB_TOKEN" \
https://api.github.com/repos/$OWNER/$REPO/actions/variables/LAST_COMMIT_HASH \
-d '{"name":"LAST_COMMIT_HASH","value":"'"$CURRENT_COMMIT_HASH"'"}'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lack empty line, could check the files after push commit (or use github desktop to help us)

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I develop on remote server with vsc remote-ssh plugin, and there is no copy on my local machine. Github desktop can not deal with the case.
Is there any recommended plugin to check problem like these?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I develop on remote server with vsc remote-ssh plugin, and there is no copy on my local machine. Github desktop can not deal with the case. Is there any recommended plugin to check problem like these?

check it manually after u push the commit:)

@aroundabout

This comment was marked as resolved.

@Radeity
Copy link

Radeity commented Sep 1, 2023

[image: image.png] It seems to have a pending label, maybe it is invisible to you? Aaron Wang @.> 于2023年9月1日周五 15:24写道:

@.
* commented on this pull request. ------------------------------ In .github/workflows/publish_latest_server_image.yml <#1 (comment)>: > + schedule: + - cron: '0 23 * * ' Hi @aroundabout https://github.com/aroundabout , may I ask why you mark this comment as resolved without any comment? Just some humble suggestion, feel free to express different opinion :D — Reply to this email directly, view it on GitHub <#1 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AL2ZYVH6SHJU2KFUR6E5ILLXYGERLANCNFSM6AAAAAA3IBHZKY . You are receiving this because you were mentioned.Message ID: @.**>

I can not see the pending msg. And in this reply, this image.png is also invisible.
image

@aroundabout
Copy link
Collaborator Author

aroundabout commented Sep 1, 2023 via email

steps:
- name: Set branch
run: |
branch=${{ inputs.branch=='' && env.branch || inputs.branch }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can choose specific branch instead of release-{tag} as the branch.
Generally, we only need to provide the tag, then the branch will be set to "release-{tag}"

.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved
.github/workflows/publish_latest_server_image.yml Outdated Show resolved Hide resolved

- name: Updata last SHA
env:
GITHUB_TOKEN: ${{ secrets.PAT }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, "PAT" is a token accessible to the repo to set public vars

Comment on lines +33 to +34
repository: ${{ env.REPOSITORY_URL }}
ref: ${{ env.BRANCH }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do not have real difference, but in Github action docs, it said that:
"Runner environment variables are always interpolated on the runner machine. However, parts of a workflow are processed by GitHub Actions and are not sent to the runner. You cannot use environment variables in these parts of a workflow file. Instead, you can use contexts. For example, an if conditional, which determines whether a job or step is sent to the runner, is always processed by GitHub Actions. You can use a context in an if conditional statement to access the value of an variable."
It mease that we can not directly use $BRANCH in these parts like "if" or "with", and we must use env context to get them. Because they are processed by github action, but not runner machine. You can not use $BRANCH to get the value. In github action, there is not $BRANCH, but only the env context or other context.
But in "run statement", these vars are sent to runner machine instead of github action, where we can use $BRANCH directly to get the value.

BTW, in "if conditional statement", use ${{ }} seems to be the recommended way, I have fixed it.

-d '{"name":"'"$var_name"'","value":"'"$current_commit_hash"'"}'
-H "Authorization: Bearer $GITHUB_TOKEN" \
https://api.github.com/repos/$OWNER/$REPO/actions/variables/LAST_COMMIT_HASH \
-d '{"name":"LAST_COMMIT_HASH","value":"'"$CURRENT_COMMIT_HASH"'"}'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I develop on remote server with vsc remote-ssh plugin, and there is no copy on my local machine. Github desktop can not deal with the case.
Is there any recommended plugin to check problem like these?

Copy link
Collaborator

@imbajin imbajin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now

REPOSITORY_URL: apache/hugegraph
BRANCH: master
IMAGE_URL: hugegraph/hugegraph:latest
GITHUB_TOKEN: ${{ secrets.PAT }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PAT means?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants