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

Update codegen image version #4970

Merged
merged 7 commits into from
Jun 14, 2024
Merged

Update codegen image version #4970

merged 7 commits into from
Jun 14, 2024

Conversation

ffjlabo
Copy link
Member

@ffjlabo ffjlabo commented Jun 13, 2024

What this PR does / why we need it:

Update codegen image version on gen/code

Which issue(s) this PR fixes:

Part of #4874
Related to #4968

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 13, 2024

gen/code failed 😇 I will investigate it.
https://github.com/pipe-cd/pipecd/actions/runs/9492831530/job/26160595802?pr=4970

Copy link

codecov bot commented Jun 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 29.33%. Comparing base (462b842) to head (b06e221).

Current head b06e221 differs from pull request most recent head f609eae

Please upload reports for the commit f609eae to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4970      +/-   ##
==========================================
+ Coverage   29.32%   29.33%   +0.01%     
==========================================
  Files         323      323              
  Lines       40984    40984              
==========================================
+ Hits        12017    12024       +7     
+ Misses      28005    27999       -6     
+ Partials      962      961       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Makefile Outdated
@@ -199,7 +199,7 @@ update/copyright:
.PHONY: gen/code
gen/code:
# NOTE: Specify a specific version temporally until the next release.
docker run --rm -v ${PWD}:/repo -it --entrypoint ./tool/codegen/codegen.sh ghcr.io/pipe-cd/codegen@sha256:fc3db505ef8dbf287b90aafed8c28246d2cca06bda2b43893a3059719fe9fff4 /repo #v0.44.0-38-g7229285
docker run --rm -v ${PWD}:/repo -it --entrypoint ./tool/codegen/codegen.sh ghcr.io/pipe-cd/codegen:v0.47.3-rc0-1-g7e27cad /repo
Copy link
Contributor

Choose a reason for hiding this comment

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

How about specifying the image with sha256 hash, as specified in the PR's base commit?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see.
Is the sha256 hash better than specifying version?
I don't have a strong opinion, but I felt redundant hash with version comment like the base commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

The image specified with the tag can be overwritten, such as the git tag.
Conversely, the sha256 hash is immutable, such as the git commit hash.
So, specifying with the sha256 hash is more reproducible.

Copy link
Member Author

Choose a reason for hiding this comment

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

I got your point! Thank you! I'll fix it.

@khanhtc1202
Copy link
Member

@ffjlabo You have to update this line in order to make the CI passed
https://github.com/pipe-cd/pipecd/blob/master/.github/workflows/gen.yaml#L14

@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 13, 2024

oops sorry 🙏

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 13, 2024

Workflow still failed 🤔
https://github.com/pipe-cd/pipecd/actions/runs/9492993824

docker://ghcr.io/pipe-cd/codegen@sha256:277a911717e5bd33aca7b38f05dd8ab4a047ba5038a81f9b43d816a5a921fe84 is not allowed to be used in pipe-cd/pipecd. Actions in this workflow must be: within a repository that belongs to your Enterprise account or matching the following: actions/cache@v2, actions/setup-go@v2, Azure/setup-helm@v1, actions/setup-node@v3, peaceiris/actions-hugo@c03b5dbed22245418539b65eb9a3b1d5fdd9a0a6, softprops/action-gh-release@1e07f4398721186383de40550babbdf2b84acfc5, actions/labeler@v4, reviewdog/action-golangci-lint@f3dc5fadcaff5d8da3574b129a58db433171b1a8, actions/checkout@v3, docker/login-action@49ed152c8eca782a232dede0303416e8f356c37b, actions/setup-go@v3, contributor-assistant/github-action@b3bbab0a75fa27270069933cec6f369c0b373b4e, pipe-cd/actions-gh-release@v2.4.0, github/codeql-action/analyze@v2, github/codeql-action/autobuild@v2, github/codeql-action/init@v2, codecov/codecov-action@v3, docker/setup-qemu-action@8b122486cedac8393e77aa9734c3528886e4a1a8, docker...

docker://ghcr.io/pipe-cd/codegen@sha256:277a911717e5bd33aca7b38f05dd8ab4a047ba5038a81f9b43d816a5a921fe84 is not allowed to be used in pipe-cd/pipecd. Actions in this workflow must be: within a repository that belongs to your Enterprise account or matching the following

Maybe need to fix the actions setting for the repo

@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 13, 2024

I tried to add the docker://ghcr.io/pipe-cd/codegen@sha256:277a911717e5bd33aca7b38f05dd8ab4a047ba5038a81f9b43d816a5a921fe84 to the Actions permissions in the setting.

So lets recheck.

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 13, 2024

CI failed because there is the diff in pkg/app/server/service/webservice/service.pb.auth.go.

https://github.com/pipe-cd/pipecd/actions/runs/9493373712/job/26161960073?pr=4970

HEAD detached at pull/4970/merge
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   pkg/app/server/service/webservice/service.pb.auth.go

Also encountered the problem on my local machine using the image 🤔

khanhtc1202
khanhtc1202 previously approved these changes Jun 13, 2024
@khanhtc1202
Copy link
Member

So with the new codegen image, the generated auth file still not the same as last time succeed 🤔 Which diff is generated? 👀

@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 13, 2024

@khanhtc1202
This is the diff. Some methods disappear 🤔
less

@khanhtc1202
Copy link
Member

So this issue remained 🥹
#4963

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
@ffjlabo
Copy link
Member Author

ffjlabo commented Jun 14, 2024

CI gen passed!
https://github.com/pipe-cd/pipecd/actions/runs/9509753883/job/26213180311?pr=4970

Comment on lines 13 to 15
- name: check CPU architecture
shell: bash
run: cat /proc/cpuinfo && uname -a
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I forgot to remove this.
removed b06e221

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Comment on lines 13 to 15
- name: check CPU architecture
shell: bash
run: cat /proc/cpuinfo && uname -a
Copy link
Contributor

Choose a reason for hiding this comment

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

This step is for debugging?
We can add this on demand, so we don't have to include this step in the usual run.

Copy link
Contributor

Choose a reason for hiding this comment

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

oops, you already removed this. thank you.

Copy link
Contributor

@Warashi Warashi left a comment

Choose a reason for hiding this comment

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

Thank you!

@ffjlabo ffjlabo requested a review from khanhtc1202 June 14, 2024 02:47
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Here you go 👍

@ffjlabo ffjlabo merged commit 6b8c6e2 into master Jun 14, 2024
14 of 16 checks passed
@ffjlabo ffjlabo deleted the update-codegen-image-hash branch June 14, 2024 02:52
dgannon991 pushed a commit to dgannon991/pipecd-pipecd that referenced this pull request Jun 20, 2024
* Update codegen image version

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Update codegen image hash in the CI

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Fix to use image hash in Makefile

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* For checking cpu arch

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Fix codegen image hash

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Remove checking cpu arch

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

---------

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: David Gannon <19214156+dgannon991@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants