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

Releasing versioned (base) images #10827

Merged
merged 144 commits into from
Sep 20, 2024
Merged

Conversation

poikilotherm
Copy link
Contributor

@poikilotherm poikilotherm commented Sep 6, 2024

What this PR does / why we need it:
We want to release versioned tags of the base image (and eventually the application images, too).
This requires setting up different versioning within the POM files, a better maintenance workflow to keep images updated and coordination between connected workflows.

Which issue(s) this PR closes:

Special notes for your reviewer:
Open TODOs to call this done:

  • Application image push workflow needs a tag on PRs, as it can't use the input var from workflow_call
  • Needs testing if it works with the PR comment (needs a PR within the fork to test this without merging here first)
  • Move Docker Hub image description README update for gdcc/base to maintenance workflow, also listing the supported tags
  • Finalize PR after all testing etc is done by switching all the action references and many other pieces to match the upstream IQSS (see TODO annotations in the code)
  • Add release note
  • Optional: add path filtering as annotated to avoid duplicate runs and race conditions
  • Optional: add app image builds directly into the maintenance workflow (might be a separate PR within scope of Tags for application container images #10618 )

Suggestions on how to test this:
All testing can only be done in the fork, as most of the workflows must be on the default branch to test them.
This has been done thoroughly, but there may be other non-testable edge cases we will only trip over once merged.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No

Is there a release notes update needed for this change?:
Probably.

Additional documentation:

grafik
grafik

Preview docs at https://dataverse-guide--10827.org.readthedocs.build/en/10827/container/base-image.html

For scheduled maintenance, we will provide updates to the last three base images as well as the develop branch.
Checkout the branch/tag we receive from the discovery job
…from Maven

Using Maven properties, we find out which is the projects preferred Java version. We don't need a build matrix here.
We no longer work on PRs in this workflow, so we don't need to safeguard against not running steps in this case.
We no longer work on PRs in this workflow, so we don't need to safeguard against not running steps in this case.

Also we no longer use the static image names, but rely on the truth Maven has in the container-base module about the version of the image.
Fix jq output by removing newlines and superfluous whitespace.
…cceed

We use the branch name from the matrix as input argument. It's not being used yet.
Deleted by accident when working on alternativ matrix way to build app image after base.
- Enable -Ddocker.noCache for BuildX
- Hopefully straightens out some bugs on Darwin with M1/M2
This way we have a distinct path and name to inject a BuildX builder instance configuration
Before, we would have run the workflow for develop only. In case develop has merges before the run starts because of a tag being created, this may result in undesired side effects.

Keep in mind that pushes of tags here will also trigger releasing a new application image!
Re-enable and change everything necessary to reference the upstream IQSS context as of now.
Without this in case of the pull_request event the input is null and the build fails because we have base image defined at all.

Simply not adding the option if the input is undefined means we stick to what is defined within the POM.
@poikilotherm poikilotherm marked this pull request as ready for review September 19, 2024 21:00
@pdurbin pdurbin assigned pdurbin and unassigned poikilotherm Sep 19, 2024
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just a tiny comment below.

pom.xml Show resolved Hide resolved
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I don't expect this to cause any problems for Dataverse itself (no Java code was touched). I'm merging this in the interest of science and progress. We can back it out if need be. 🚀

@pdurbin pdurbin merged commit 1d31fb0 into IQSS:develop Sep 20, 2024
12 of 15 checks passed
@pdurbin pdurbin added this to the 6.4 milestone Sep 20, 2024
@pdurbin pdurbin removed their assignment Sep 20, 2024
bjosel added a commit to TIK-NFL/dataverse that referenced this pull request Nov 19, 2024
* missing empty watermark entry

* fix capitalization

* Changed: dataverse image_url Solr property set on SearchServiceBean

* Changed: do not modify existing JSF logic

* remove unused imports IQSS#10517

* add test to assert capitalizataion of Dataset and Software IQSS#10517

* add details to error messages (IQSS#10813)

* Fix addDataverse expected request body structure (IQSS#10802)

* Fixed: MetadataBlockServiceBean to check for not excluded fields in input levels

* Changed: using queries for obtaining dataset field types based on displaying conditions

* Refactor: json printer method for MetadataBlock

* Added: IT test case for list metadata blocks testing field with include=false and displayOnCreate=true property

* Fixed: removed condition in MetadataBlockServiceBean

* Added: release notes for IQSS#10741

* Fixed: displayOnCreate query logic

* Fixed: excluding conditionally required fields when display-on-create is true

* Fixed: query predicate for required-in-dataverse field condition

* Fixed: addDataverse API facetIds field json structure

* Added: docs IQSS#10800

* A one line fix for IQSS#10821 - ? (IQSS#10823)

* Add thumbnail for featured dataverses (IQSS#10433)

* Add thumbnail for futured dataverses

* Add documentation

* Release note snippet

* New flyway namming

* Update doc/release-notes/10433-release-notes.md

Co-authored-by: Philip Durbin <[email protected]>

* Release note snippet update
Add new recommandations (HTML preview + "for more information ...")

* Update SQL file name after 6.2 release

* renamed sql file

---------

Co-authored-by: Philip Durbin <[email protected]>
Co-authored-by: Ludovic DANIEL <[email protected]>
Co-authored-by: Philip Durbin <[email protected]>

* bump sql script version IQSS#10517

* JDD Metrics: Label KO IQSS#10123 (IQSS#10124)

* remove parentheses

* Correction of the parenthesis display

* conditional INSERT of dataset type IQSS#10517

* Add logic to suppress query tool display for non-public files.

* typo

* fix test

* fix labels when cvoc is used

* doc tweaks for versioned base images: making releases IQSS#10827

* iterate on "supported image tags" section IQSS#10827

* Added: setting imageUrl in SearchServiceBean for datasets and files

* simplify now that everything is inside the try

* update tests - added one field in citation block

* reworked controlled vocab language keys

* fixing key to lowercase

* fixing key to lowercase

* release note

* undo changes

* Update doc/release-notes/10810-search-api-payload-extensions.md

Co-authored-by: Philip Durbin <[email protected]>

* Added: note about upcoming change to image_url field in docs

* fixing language list

* fixing language list

* fixing language list

* fixing language list

* fixing language list

* fixing language list

* changes per review comments

* changes per review comments

* support no pubIdType for URLs

* direct people to the log for failures - they aren't in the response

* bug - the _target url isn't being set elsewhere

* avoid failing when the entity is null for error statuses

* don't update unpublished files - no need and it will fail

the updateIdentifier call is checking for the findable metadata which is
not available before publication.
(We don't update DataCite after dataset edits, so unpublished datasets
don't go through here, but unpublished files on published datasets would
hit this code)

* lower logging, add null check on relatedIdentifier

* Change to use POST for all

* Documentation and updated release note

* changes per review comments

* test fix - number of fields

* Remap oai_dc fields dc:type and dc:date (IQSS#10737)

* Remap oai_dc fields dc:type, dc:date, and dc:rights IQSS#8129.

The `oai_dc` export and harvesting format has had the following fields remapped:

- dc:type was mapped to the field "Kind of Data". Now it is hard-coded to the word "Dataset".
- dc:date was mapped to the field "Production Date" when available and otherwise to "Publication Date". Now it is mapped only to the field "Publication Date".
- dc:rights was not mapped to anything. Now it is mapped (when available) to terms of use, restrictions, and license.

* add tests for export and citation date IQSS#8129

* map dc:date to pub date or field for citation date  IQSS#8129

* back out of any changes to dc:rights IQSS#8129

* remove OAI-PMH changes from API changelog (also in release note) IQSS#8129

* tweak release note, mention backward incompatibility, reexport IQSS#8129

* update release note

* check for ROR in grantAgency field too

* 10527 404static.xhtml has an old date in the footer (2023 is hard-coded) and update URLs (IQSS#10535)

* Javascript updates the year automatically and the URLs are customised.

* add taps

* only change year and and all the other errors are still there.

* adopt using CDI, fix funderIdentifier element per schema

* datasetTypes test fix

* release note/changelog changes

* chore(ct): update base image wait4x to 2.14.2 for stdlib update

* chore(ct): update base image wait4x to 2.14.2 for stdlib update IQSS#10844

* don't send contributors w/o contributorType

* relatedIdentifierType is required

* flip to prefer identifier over url

seeing cases at QDR where the type is DOI, the identifier is the doi and
the URL is a non-DOI reference (e.g. at pubmed).

* Handle case where type is set but there's no identifier

* map non-standard contributors to Other, remove unused imports

* Treat missing contrib type as Other

* chore(ci): delete duplicate action after renaming

* docs(ct): update base image tag policy from latest discussion IQSS#10827

As discussed during the 2024-09-12 containerization working group meeting (see ct.gdcc.io) and on https://dataverse.zulipchat.com/#narrow/stream/375812-containers/topic/change.20version.20scheme.20base.20image.3F/near/469884104

* style,docs(ct): minor tweaks to base image policy as per @pdurbin

* feat(ct): switch latest to unstable in base image flow

As per latest discussion, we want to keep the unstable tag around.
It shall still point to the latest from develop as it has been done
before our revised tagging policy.

Latest will be used for production images, much more aligned with the
Bitnami policy

* style,docs(ct): minor tweaks to base image policy as per @pdurbin

* fix(ct): remove auth for revision action

Maybe we won't run into a rate limit for now, as the limit of 180/s * 100 = 18000 tags/s seems to leave us some headroom for now.

* fix(ct): try to avoid shell substitution in base image flow

We might need more backslashes to avoid telling the shell to replace what we want to be a maven property

* fix(ct): avoid shell substitutions going awry for base image release tag

Instead of using a Maven reference and dealing with escaping of $ chars, override the suffix with an empty string

* add trailing / after hostname for perma base-url IQSS#10775

* ci(ct): remove out of scope actions for IQSS#10478

We will deal with shipping the updated application container images separately as part of issue IQSS#10618.

Adding some comments about why some stuff is still around.

* ci(ct): reorganise tags for develop branch IQSS#10478

Now adding the "upcoming" tag during the develop branch run.

Also some reorganizing to ship the tag options using outputs, not env vars.
Moving the common Maven option to enable tag overrides to the command instead of the options building.

* chore(parent): remove OSS plugin snapshot repo after upgrade to DMP 0.45

* build(ct): make application image use new base image flexi stack IQSS#10478

* style(ct): remove stale comment from base module pom

* chore(ct): add comment about apt sec updates detection via list hack

For now, we stick to not alter the image more than necessary. Only packages we installed will be upgraded, as these are not part of the normal Java base images.

The Java base images receive regular updates and undergo testing. It might be unwise to just install all the security updates we could get.

Leaving the option here for later saves the trouble to dig up the solution again.

* ci(ct): use new setup-maven action in base image push workflow

Simplify setup following DRY principle

* ci(ct): replace logic in base image push workflow

Using the same actions and steps as done in the maintenance action should work for this workflow in case of a push event, too.

* test(ct): temporarily set dev branch in base image push flow to feature branch

* ci(ct): use setup-maven action in app image push workflow to simplify setup

Following DRY principle, reuse the steps defined

* test(ct): temporarily enable app image push flow execution in all forks

* avoid spurious log warning for others e.g. isbn

these were going through the default check for URLs and failing (not a
url) leading to a warning. The new code should try URL parsing for URLs,
try PID and URL parsing for ones with no type specified, and send the
rest of the identifiers w/o any additional (optional) attributes.

* update doc

* ci(ct): use an optional base image ref for app image push flow

We need to transfer the determined base image name we might have just built from the calling workflow into this flow.
As we provide a default value, this is picked up for pull_requests.

* ci(setup-maven): try to auto-detect git ref

It's not so easy to determine the right git ref for different scenarios like PR, etc. Unless explicitly given a ref, try to autodetect the right one to go with.

* chore(ct): add notes in flows about adding a path filter

We want to avoid duplicate runs which might trigger race conditions for image shipments.

* chore(ct): add note about missing triggers for base push flow

This is out of scope for IQSS#10827, but should be addressed at a later point to avoid duplicated runs with potential race conditions. Also it enables proper rebuilds for preview images when someone is just trying to create a base image change, but which should obviously be tested full chain.

* fix,ci(ct): don't trigger the base push flow for backports

Obviously these are meant for the maintenance workflow, not the push flow!

* ci(ct): trigger app flow from base push flow

We detect the tag we have been using in a finalizing step to hand a proper base image ref to the app image workflow to make it work on the images we just pushed to the registry.

* changes per review

* Apply suggestions from code review

Co-authored-by: Philip Durbin <[email protected]>

* cleaner formatting

* minor doc tweak IQSS#10632

* standardize image url

* No longer needed with use of CDI.current() in XMLMetadataTemplate

* no longer used and CrossRef ended up using it's own.

* add more info about the scope of changes.

* doc changes

* doc changes

* ci(ct): reshape maintenance workflow into external matrix script

Unfortunately, matrix jobs logs and outputs cannot be aggregated in Github Actions.

The only way to work around the limitations of GHA is by using a custom build script that create a similar matrix like experience.

This commit introduces these scripts, probably also making some custom actions we added obsolete.

* ci(ct): remove obsolete actions for revisions and parent image changes detection

* ci(ct): re-enable forced build for maintenance workflow

* doc changes

* doc changes

* ci(ct): add outputs to maintenance matrix job

Can be picked up by other jobs, e.g. to create textblocks for docs or a job matrix.

* ci(ct): remove draft of building app images in maintenance matrix job

* style,ci(ct): reword the maintenance build workflow name

* ci(ct): make the maintenance workflow push the hub description for the base image IQSS#10478

* style(ct): fix simple typo in base image README

* fix(ct): remove bug from package upgrade detection in maintenance workflow

We did not correctly compare the status code of the grep command, breaking the update detection

* update query per review comments to handle all cases

* docs,style(ct): small rewording about immutable tags for base image

* feat,ci(ct): add immutable tags to list of base image tags in maintenance job

As discussed during community meeting on 2024-09-19.

* style,ci(ct): add some more verbosity about progress in maintenance job

* refactor,ci(ct): finishing touches for IQSS#10478

Re-enable and change everything necessary to reference the upstream IQSS context as of now.

* add docs for disable-dataset-thumbnail-autoselect IQSS#10819 IQSS#10820

* create 6.4 release notes and add about half the updates IQSS#10853

* add second half of snippets IQSS#10853

* fix,ci(ct): only add base image Maven option when the input is defined

Without this in case of the pull_request event the input is null and the build fails because we have base image defined at all.

Simply not adding the option if the input is undefined means we stick to what is defined within the POM.

* add highlights and upgrade steps IQSS#10853

* docs(ct): add release note for maintenance workflow IQSS#10478

* style(ct): add comment explaining what a flavor is in base image Maven props

Co-authored-by: Philip Durbin <[email protected]>

* A potential simple fix for IQSS#10667 ?

* Update 6.4-release-notes.md

corrected the schema.xml instructions

* add blurb for tagged base images IQSS#10853

* doc how to handle develop into develop PRs IQSS#9508

* remove extra line

* dont return image_url if there is none

* Fix NPE using CVOC

* set dataset type before registering pid (which needs the type)

* add release note

* adding fix from review comment

* remove tabs to make reviewdog happy. woof! IQSS#10623

* globus doc tweaks IQSS#10623

* add more and better DataCite export IQSS#10853

* add blur for listing feature flags IQSS#10853

* add CVOC bug fix IQSS#10853
-6.4-release-notes

* update image_url IQSS#10853

* Updated the docs to reflect the new name of a JVM option (IQSS#10623)

* I fixed anchor links IQSS#10876 (IQSS#10877)

* improve release note IQSS#10623

* add globus async IQSS#10853

* add cvoc update IQSS#10853

* add new globus settings under settings section IQSS#10853

* various tweaks IQSS#10853

* typo IQSS#10853

* put features before bug fixes IQSS#10853

* reword

* datacite title

* croissant update IQSS#10853

* bump version to 6.4 IQSS#10852 (IQSS#10871)

* displayOnCreate set to true for depositor and dateOfDeposit in Citation metadata block (IQSS#10884)

* Changed: displayOnCreate set to true for depositor and dateOfDeposit in citation.tsv

* Changed: MetadataBlocksIT test assertion for new total number of displayOnCreate fields

* Added: release notes for IQSS#10850

* Added: minor tweak to release notes

* IQSS#10853 fix typo version number

* Add release note change for fields depositor and dateOfDeposit in the citation.tsv

* remove old release note

* formatting fix

fixed formatting of the shell block in the upgrade instruction

* tweak depositor and dateOfDeposit IQSS#10853

* fixed update-fields.sh url 

(it had "9.4.1" in it; and we probably don't want to get it from the master branch either)

* reindex instruction

* removed a superfluous command line

* temp dir cleanup

* typo

* docs: update release notes from IQSS#10343

* tweaks IQSS#10343

* Upgrade to upstream version 6.4

* Merge upstream v6.4 into branch properties

* Sync with upstream

* Fix merge of properties

* bugfix: metadataFragment.xhtml

* fix relationType display value bug

---------

Co-authored-by: Jim Myers <[email protected]>
Co-authored-by: GPortas <[email protected]>
Co-authored-by: Philip Durbin <[email protected]>
Co-authored-by: landreev <[email protected]>
Co-authored-by: jeromeroucou <[email protected]>
Co-authored-by: Philip Durbin <[email protected]>
Co-authored-by: Ludovic DANIEL <[email protected]>
Co-authored-by: sbondka <[email protected]>
Co-authored-by: Stephen Kraffmiller <[email protected]>
Co-authored-by: Steven Winship <[email protected]>
Co-authored-by: Benedikt Kruse <[email protected]>
Co-authored-by: Oliver Bertuch <[email protected]>
Co-authored-by: Oliver Bertuch <[email protected]>
Co-authored-by: qqmyers <[email protected]>
Co-authored-by: paulboon <[email protected]>
Co-authored-by: ofahimIQSS <[email protected]>
Co-authored-by: Florian Fritze <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Containers Anything related to cloudy Dataverse, shipped in containers. Size: 3 A percentage of a sprint. 2.1 hours.
Projects
Status: Important
Status: Done 🧹
Development

Successfully merging this pull request may close these issues.

Tag version for the container base image
3 participants