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

GitHub action #5361

Merged
merged 34 commits into from
Dec 16, 2022
Merged

GitHub action #5361

merged 34 commits into from
Dec 16, 2022

Conversation

msciabarra
Copy link
Contributor

Implements a port of the travis build for github actions

Description

This adds a Github action workflow and a port of the travis scripts to work with GitHubactions

Travis needs to be replaced

Related issue and scope

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Scheduler
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

@msciabarra
Copy link
Contributor Author

Please note it is still incomplete because I have no idea of how to upload logs and how to see the results.

@codecov-commenter
Copy link

codecov-commenter commented Dec 8, 2022

Codecov Report

Merging #5361 (7fbcb1b) into master (8578887) will decrease coverage by 24.09%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #5361       +/-   ##
===========================================
- Coverage   81.46%   57.36%   -24.10%     
===========================================
  Files         240      240               
  Lines       14407    14407               
  Branches      618      601       -17     
===========================================
- Hits        11736     8264     -3472     
- Misses       2671     6143     +3472     
Impacted Files Coverage Δ
...a/org/apache/openwhisk/common/ConfigMapValue.scala 0.00% <0.00%> (-100.00%) ⬇️
...penwhisk/core/database/cosmosdb/CosmosDBUtil.scala 0.00% <0.00%> (-100.00%) ⬇️
...nwhisk/core/database/cosmosdb/CosmosDBConfig.scala 0.00% <0.00%> (-100.00%) ⬇️
...hisk/core/database/cosmosdb/ReferenceCounted.scala 0.00% <0.00%> (-100.00%) ⬇️
...core/database/cosmosdb/RxObservableImplicits.scala 0.00% <0.00%> (-100.00%) ⬇️
...re/database/cosmosdb/CollectionResourceUsage.scala 0.00% <0.00%> (-100.00%) ⬇️
...ore/database/cosmosdb/cache/CacheInvalidator.scala 0.00% <0.00%> (-100.00%) ⬇️
...e/database/cosmosdb/cache/ChangeFeedConsumer.scala 0.00% <0.00%> (-100.00%) ⬇️
...hisk/core/database/mongodb/MongoDBViewMapper.scala 0.00% <0.00%> (-98.89%) ⬇️
...apache/openwhisk/core/database/LimitsCommand.scala 0.00% <0.00%> (-98.51%) ⬇️
... and 144 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

.github/workflows/unit.yaml Outdated Show resolved Hide resolved
.github/workflows/performance.yaml Outdated Show resolved Hide resolved
@style95
Copy link
Member

style95 commented Dec 10, 2022

Logs are uploaded to IBM Bluemix.
https://github.com/apache/openwhisk/blob/master/tools/travis/box-upload.py#L46

We need to see if this is still valid.
I also have no access to the logs.

@dgrove-oss, could you confirm if we can still use Bluemix to upload logs?

@sciabarracom
Copy link
Contributor

sciabarracom commented Dec 10, 2022 via email

@msciabarra
Copy link
Contributor Author

I updated the pull requests.

It is now able to write on slack and upload logs to an S3 buckets.
There is also an "on demand" task allowing to launch a single test and debug the results enabling and accessing the vm with ngrok.

To use the PR we need to merge it and also add to GitHub all the secrets listed in the .github/workflows/README.

@msciabarra msciabarra changed the title GitHub action [WIP] GitHub action Dec 11, 2022
@style95
Copy link
Member

style95 commented Dec 12, 2022

How can we test this change?
It seems the GitHub workflow is not running on this repo.

@msciabarra
Copy link
Contributor Author

The workflow does not run and it is not visibile until it is available on the default branch so it shoud be merged. However currently the do not run unless you trigger them manually or push a tag. So we can test them merging and triggering manually. Or you can merge them in a fork as I am doing here (https://github.com/msciabarra/openwhisk/)

ansible/group_vars/all Outdated Show resolved Hide resolved
…aming S3_LOG_BUCKET to AWS_BUCKET to fix tests
@msciabarra
Copy link
Contributor Author

I fixed the error in the aws storage renaming a secret to AWS_BUCKET, and the error in system forcing a corresponding java version. I had to "slightly" update the version of openj9 used to use one that can be actually downloaded (11.0.8 -> 11.0.12)

@msciabarra
Copy link
Contributor Author

Now I have unit, multiruntime and standalone test passing and system only failing one test

@msciabarra
Copy link
Contributor Author

Screenshot 2022-12-13 at 20 37 53

The build is able to pass all the tests on 1.0.0 so I assume it can be merged.

Copy link
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

It seems this change is aligned with 1.0.0.
Is this will be updated to be aligned with the current master?

ansible/group_vars/all Outdated Show resolved Hide resolved
@msciabarra
Copy link
Contributor Author

more cleanup added, removed all the changed files to fix 1.0.0, now triggered by push and pull-requests without tags

@bdoyle0182
Copy link
Contributor

To Dom's comment on aligning with master past 1.0.0 and the email sent to the dev list, I don't think master is broken. The CI tests should all pass, I think there was some flakiness with travis / the scheduler tests that would require you to re-kick the travis job to get them to pass; but master should be in a fine state.

Thank you so much for the effort on this before the holidays.

@msciabarra
Copy link
Contributor Author

msciabarra commented Dec 14, 2022

Yes, I confirm the master is NOT broken. Indeed I was able to run all the master tests against my fork. At this point I think it is safe to merge.

Screenshot 2022-12-14 at 17 59 47

@msciabarra msciabarra closed this Dec 14, 2022
@msciabarra msciabarra reopened this Dec 14, 2022
@msciabarra
Copy link
Contributor Author

Sorry my mistake. Please merge now. I tested the build against my fork and it works.

@bdoyle0182 bdoyle0182 closed this Dec 14, 2022
@bdoyle0182 bdoyle0182 reopened this Dec 14, 2022
Copy link
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

Thanks, @msciabarra
This is great work!

Generally LGTM with minor questions.

.github/workflows/0-on-demand.yaml Show resolved Hide resolved
SLACK_WEBHOOK: ${{secrets.SLACK_WEBHOOK}}

# (optional) s3 log upload
AWS_BUCKET: ${{ secrets.AWS_BUCKET }}
Copy link
Member

Choose a reason for hiding this comment

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

Same for NGROK, so now contributors are in charge of uploading logs to their own environment to figure out the failed tests of a PR.
For periodic tests against the upstream master branch, those who have permission can configure proper secrets for the upstream repo.

It needs to be discussed where to place such logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you can put your own AWS_BUCKET where you want. The bucket can be made public as I did here: https://nuvolaris-logs.s3.eu-central-1.amazonaws.com/index.html.
Instructions are in the README.
Actually, since there is already a bucket configured for some tests, it will be used!
Maybe this is not what the owner of the bucket wants.
We need to get a bucket from Amazon or from some other cloud provider, I am sure there are donations somewhere of resources that we can use...


If you want to get notified of what happens on slack, create an [Incoming Web Hook](https://api.slack.com/messaging/webhooks) and then set the following secret:

- `SLACK_WEBHOOK`: the incoming webhook url provided by slack.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is also same. It would be great to discuss how/whether to configure this for the upstream repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can use it with the current OpenWhisk slack. I do not know who has access to the slack configuration. All you need to do is to create an app and retrieve the incoming webhook. And you can use the exiting dev channel on OpenWhisk slack

@@ -0,0 +1,26 @@
#!/usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to test the build without having to run a full test... it is handy to debug secrets and so on

# limitations under the License.
#

#if [[ $TEST_SUITE =~ Dummy ]]
Copy link
Member

Choose a reason for hiding this comment

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

This might need to be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, if you do not want to defeat the goal of the dummy test to avoid to run a fast build withou waiting an hour...

Copy link
Member

Choose a reason for hiding this comment

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

I love this idea to skip tests for debugging.
How about making this as an input as well just like debugging enablement option so that users can easily trigger this feature without changing any code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is already there...if you launvh the dummy build and enable ngrok in a few second you get an ssh command to get into the vm with everythingalready setup to build and test.

Copy link
Member

Choose a reason for hiding this comment

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

Can we control whether to run these dummy tests like this?
image

Also, there is the same code block, so can we delete these comments?
https://github.com/apache/openwhisk/pull/5361/files#diff-fc5257f5200a46b6baab3ca657b694cc84dc77e3a5f9d71062b25fce994c02b4R70

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you control the test from there. Select "Dummy" to run the dummy test. It is useful when also you enable Ngrok debugging so you get an ssh command to get into the CI build VM to experiment.

Copy link
Member

@style95 style95 Dec 16, 2022

Choose a reason for hiding this comment

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

ah ok, I got confused.
My last question is what is the purpose of this commented block?
There is already uncommented one.

if [[ $TEST_SUITE =~ Dummy ]]
then echo skiping setup ; exit 0
fi```

tools/github/setup.sh Show resolved Hide resolved
tools/github/runDummyKOTests.sh Outdated Show resolved Hide resolved
@@ -15,6 +15,7 @@
* limitations under the License.
*/

/*
Copy link
Member

Choose a reason for hiding this comment

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

Why is this test commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was an attempt to pass the travis build to merge... but looks like it does not work

# limitations under the License.
#

#if [[ $TEST_SUITE =~ Dummy ]]
Copy link
Member

Choose a reason for hiding this comment

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

Can we control whether to run these dummy tests like this?
image

Also, there is the same code block, so can we delete these comments?
https://github.com/apache/openwhisk/pull/5361/files#diff-fc5257f5200a46b6baab3ca657b694cc84dc77e3a5f9d71062b25fce994c02b4R70

@msciabarra
Copy link
Contributor Author

Definitely ready to merge. This is what I get on my fork:
Screenshot 2022-12-16 at 10 52 51
All tests are passing.

@sciabarracom sciabarracom merged commit f717619 into apache:master Dec 16, 2022
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.

None yet

6 participants