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 actions - Activating extra profiles #2987

Merged
merged 3 commits into from
Mar 12, 2020

Conversation

pmauduit
Copy link
Member

@pmauduit pmauduit commented Mar 11, 2020

This PR actually aims to improve some of the github actions workflows:

  1. removing the M2_SETTINGS_FILE secret: there is actually nothing secret about it (see actions - remove the need for a M2_SETTINGS_FILE secret #2985 )
  2. limit docker stuff to the main geOrchestra repository: There is no need to consume CPU resources if we don't do anything with the docker image after the build. Following something similar to the previous issue: we likely to have no secrets configured on the fork to push the generated docker images.
  3. Enable sentry-spring and jsonevent-layout m2 profiles when generating the official docker images (the actual purpose of this PR). The overhead on the generated artifact is less than 1MB, so even if it is not used by the final user, it seems to provide an interesting added value to the webapps, allowing to plug them to external tools (sentry, logstash/kibana and so on).

There is nothing actually secret into it, the forks which also run the
gh actions could also benefit from this configuration.
1. fork won't likely to have the necessary secrets to push

2. docker images are built with tests disabled, if nothing is done (e.g
saving the images somewhere), then they will lost at the end of the
build and we are just wasting computing resources. There is no added
value of building them after the initial build.
Note:

This will add optional dependencies to the generated docker images. The
overhead is ~ 750 KB, so even if these dependencies are not used at
runtime, we consider that the size increase is acceptable, compared to
the artifact actual size.

For the record:

* log4j-logstash adds the jsonevent-layout log4j1 dependency, which
allows to have a "jsonified" output, which is interesting in case of
parsing the logs with tools like logstash.

* sentry-spring will add the following ones:

```
[INFO] \- io.sentry:sentry-spring:jar:1.7.27:compile
[INFO]    \- io.sentry:sentry:jar:1.7.27:compile
```

And allows the webapp to be connected against a sentry instance. See
https://docs.sentry.io/clients/java/integrations/#spring for
the configuration details.
@RemiDesgrange
Copy link
Contributor

👍 for me

@fvanderbiest
Copy link
Member

Probably a lighter syntax can be achieved with https://github.com/marketplace/actions/cancel-this-build

@fvanderbiest fvanderbiest self-requested a review March 12, 2020 08:02
@pmauduit
Copy link
Member Author

pmauduit commented Mar 12, 2020

Probably a lighter syntax can be achieved with https://github.com/marketplace/actions/cancel-this-build

I think we still want the test / verify goals actions from maven being run in the forks. My strategy had been to basically "skip the step whenever it needs a secret". I'm curious to know what the suggested action does exactly (which state will get the job in case of cancellation ?)

@pmauduit
Copy link
Member Author

I'm curious to know what does the suggested action does exactly (which state will get the job in case of cancellation ?)

It's using the github actions API to cancel the job, so the job will get a cancelled status, which is not exactly what we are willing to do.

@fvanderbiest fvanderbiest merged commit c05a72b into master Mar 12, 2020
@fvanderbiest fvanderbiest deleted the activating-extra-profiles branch March 12, 2020 10:36
@fvanderbiest
Copy link
Member

@fvanderbiest fvanderbiest added this to the 20.1.0 milestone Mar 12, 2020
@fvanderbiest
Copy link
Member

Should be backported to 20.0.x branch too

@pmauduit
Copy link
Member Author

Please also consider porting to https://github.com/georchestra/geonetwork/blob/georchestra-gn3.8.2-master/.github/workflows/geonetwork-docker.yml

I cannot see the point of even activating the github-actions for forks on the georchestra/geonetwork repository: the tests are skipped.

@pmauduit
Copy link
Member Author

Also, there are no sentry-spring maven profile in the GeoNetwork fork

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

3 participants