Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

GitHub Actions for CI #1597

Merged
merged 3 commits into from
Mar 13, 2020
Merged

GitHub Actions for CI #1597

merged 3 commits into from
Mar 13, 2020

Conversation

lbonn
Copy link
Contributor

@lbonn lbonn commented Mar 12, 2020

No description provided.

@lbonn lbonn force-pushed the ci/github-actions branch 2 times, most recently from 9326843 to 105d1d5 Compare March 12, 2020 15:47
@lbonn
Copy link
Contributor Author

lbonn commented Mar 12, 2020

Caching should already work (after a merge for master for docker).

Right now, codecov has been scraped, I'll see if it's easy to restore.

Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

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

Looks pretty good!

What is the goal of the docker workflow? Are we not already updating those images in gitlab?

Are these two log messages particularly concerning?

WARNING! Your password will be stored unencrypted in /home/runner/.docker/config.json.

And:

Error response from daemon: repository docker.pkg.github.com/advancedtelematic/aktualizr/aktualizr-ci not found: name unknown: docker package "aktualizr-ci" does not exist under owner "advancedtelematic/aktualizr"

It's also weird that I don't see the output of the tests in the coverage run. I guess if it succeeds, it's fine.

Actually uploading coverage would be good, but can't we move that to gitlab as well? Might be better.

@lbonn
Copy link
Contributor Author

lbonn commented Mar 13, 2020

@patrickvacek

What is the goal of the docker workflow? Are we not already updating those images in gitlab?

In gitlab CI, we store the images in gitlab's docker repository which is only accessible from the company network.
Here, we have to store them somewhere else and github conveniently has a space for that: https://help.github.com/en/packages/using-github-packages-with-your-projects-ecosystem/configuring-docker-for-use-with-github-packages

Are these two log messages particularly concerning?

WARNING! Your password will be stored unencrypted in /home/runner/.docker/config.json.

No, not really that's the recommended way (see link above).

More info on the token here: https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token.

I suspect that actions triggered from external forks will actually won't be able to use this token and won't benefit from caching. That should probably be tested as well.

Tagging @zabbal, just in case :)

And:

Error response from daemon: repository docker.pkg.github.com/advancedtelematic/aktualizr/aktualizr-ci not found: name unknown: docker package "aktualizr-ci" does not exist under owner "advancedtelematic/aktualizr"

Right now, no image has been pushed but this should go away as soon as we merge this PR to master.

It's also weird that I don't see the output of the tests in the coverage run. I guess if it succeeds, it's fine.

I've also noticed that it sometimes gives out logs very slowly or not at all. But the "View raw logs" from the menu in the corner has worked for me so far.

Actually uploading coverage would be good, but can't we move that to gitlab as well? Might be better.

I'll look into it today :).

lbonn added 2 commits March 13, 2020 11:04
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
To replace Travis

Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn lbonn force-pushed the ci/github-actions branch from 105d1d5 to 915e18b Compare March 13, 2020 10:17
@zabbal
Copy link
Contributor

zabbal commented Mar 13, 2020

Yepp, looks fine to me. Github have access to their token anyway so when it's stored in their CI runner it makes no difference from security PoV

Out of curiosity, what's the advantage of gh-actions compared to travis?

Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn lbonn force-pushed the ci/github-actions branch from 915e18b to 071e052 Compare March 13, 2020 10:43
@pattivacek
Copy link
Collaborator

I've also noticed that it sometimes gives out logs very slowly or not at all. But the "View raw logs" from the menu in the corner has worked for me so far.

Thanks for that tip!

And everything else looks/sounds fine.

@lbonn
Copy link
Contributor Author

lbonn commented Mar 13, 2020

I suspect that actions triggered from external forks will actually won't be able to use this token and won't benefit from caching. That should probably be tested as well.

I was wrong actually (https://help.github.com/en/actions/configuring-and-managing-workflows/creating-and-storing-encrypted-secrets#using-encrypted-secrets-in-a-workflow):

With the exception of GITHUB_TOKEN, secrets are not passed to the runner when a workflow is triggered from a forked repository.

But the security is here: https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token#permissions-for-the-github_token

However, this also means that codecov won't work for forked PRs if I add it here, as it uses a codecov token as a secret.

@zabbal thanks for the quick look. Travis and GitHub Actions are both free for public repositories. The advantage is that GitHub actions seems much more supported right now than Travis which looks like it's maintained on life support. It's been slower over time and it has the hard time out limit of 50 minutes that has caused problems in the past.

@codecov-io
Copy link

Codecov Report

Merging #1597 into master will decrease coverage by 1.32%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1597      +/-   ##
==========================================
- Coverage   82.44%   81.11%   -1.33%     
==========================================
  Files         189      189              
  Lines       11932    11932              
==========================================
- Hits         9837     9679     -158     
- Misses       2095     2253     +158
Impacted Files Coverage Δ
src/sota_tools/request_pool.h 11.11% <0%> (-88.89%) ⬇️
...rc/libaktualizr/package_manager/dockerappmanager.h 4% <0%> (-80%) ⬇️
src/aktualizr_secondary/update_agent_ostree.h 25% <0%> (-75%) ⬇️
src/virtual_secondary/managedsecondary.h 33.33% <0%> (-66.67%) ⬇️
src/sota_tools/ostree_object.h 40% <0%> (-60%) ⬇️
src/aktualizr_info/aktualizr_info_config.h 50% <0%> (-50%) ⬇️
src/libaktualizr/crypto/keymanager.h 50% <0%> (-50%) ⬇️
src/libaktualizr/uptane/exceptions.h 45.09% <0%> (-47.06%) ⬇️
src/libaktualizr/storage/invstorage.h 53.06% <0%> (-44.9%) ⬇️
src/sota_tools/server_credentials.h 41.17% <0%> (-35.3%) ⬇️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c4d5c0...071e052. Read the comment docs.

@lbonn
Copy link
Contributor Author

lbonn commented Mar 13, 2020

test_imagerepo_failure just failed three times in a row...

@mike-sul
Copy link
Collaborator

test_imagerepo_failure just failed three times in a row...

Maybe, it indicates that there is an issue(s) with handling edge cases of Images repo in aktualizr...
I'll look into it on Monday.

@mike-sul
Copy link
Collaborator

mike-sul commented Mar 13, 2020

Well, the following message speaks for itself
ERROR:test_fixtures:None ERROR:test_fixtures:Failed to get aktualizr info/status

@lbonn
Copy link
Contributor Author

lbonn commented Mar 13, 2020

Well, the following message speaks for itself
ERROR:test_fixtures:None ERROR:test_fixtures:Failed to get aktualizr info/status

Does it? What do you mean?

Copy link
Collaborator

@mike-sul mike-sul left a comment

Choose a reason for hiding this comment

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

Looks quite similar to the gitlab's yaml. I am just wondering if it's really free for open source projects.

Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

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

LGTM. But a couple questions:

  1. What changed since the last round of errors?
  2. Is it better to have the codecov upload here or in gitlab? I'm fine either way, so there's no problem with this as it is.

@lbonn
Copy link
Contributor Author

lbonn commented Mar 13, 2020

  1. What changed since the last round of errors?

The bad kind of "fix", pressed retry :). I've looked a little bit in the test but I'm not sure what could make it more reliable.

  1. Is it better to have the codecov upload here or in gitlab? I'm fine either way, so there's no problem with this as it is.

Opinion:

gitlab:
+ it is our canonical reference CI, more logical to put it here
- won't run for external PRs
- already stores the html coverage report in html format, though it's quite impractical to use (a zip archive), so it could be confusing?

github:
+ already done haha
- it's our backup CI
- won't run for external PRs in the current state (because it uses GITHUB_TOKEN)
~ it looks like it could work for external contributions using the official action https://github.com/marketplace/actions/codecov. I've considered it but I'm not sure all the options we use are accessible this way right now

But switching it to gitlab should be easy in the current state anyway, just a matter of adding the token there.

@lbonn lbonn merged commit 113e794 into master Mar 13, 2020
@lbonn lbonn deleted the ci/github-actions branch March 13, 2020 18:30
@lbonn
Copy link
Contributor Author

lbonn commented Mar 16, 2020

I forgot a || true in the docker caching steps, so they are blocked right now. I'll push something manually which should fix it, then open a new small PR.

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

Successfully merging this pull request may close these issues.

5 participants