DM-40418: Release Management GitHub Action #115

dspeck1 · 2024-01-18T22:22:41Z

Add GitHub Action to build releases. Playbook is updated to include release management instructions. This also moves the base Dockerfile into the base directory and changes naming of activator Dockerfile. Both the build base and build GitHub actions are updated accordingly.

kfindeisen

Sorry for the delay, it took me a while to digest things (and seeing the workflow in action on lsst-dm/next_visit_fan_out#9 was indeed useful).

My main concern is the unclear relationship between ci-release and build-service, and the policy decisions that follow from that. Is ci-release meant to replace build-service (if so, why did you create a new script from scratch instead of just injecting support for tag refs into the old one?), or is it meant to be a supplement (if so, why does it run on anything other than tags/releases)? I'd prefer the former, just to be sure that we're really building things the same way for both testing and production.

Either way, we need to clean up how latest base containers are handled before a release workflow that always uses latest is safe to merge.

.github/workflows/ci-release.yaml

kfindeisen · 2024-01-26T21:42:18Z

.github/workflows/ci-release.yaml

+      - "tickets/**"
+      - "u/**"
+    tags:
+      - "*"


Why use push:tags instead of release:published? As far as I can tell the only difference would be if somebody pushed a tag that doesn't have a corresponding release, but the latter seems more direct.

If you do keep push:tags, would it be useful to filter on tags that look like version numbers, to guard against somebody pushing non-release tags?

.github/workflows/ci-release.yaml

kfindeisen · 2024-01-26T23:06:35Z

Dockerfile

Why make a copy of Dockerfile.activator instead of renaming it (and updating build-service to match, of course)?

The name of the Dockerfile is not an input parameter to the composite GitHub Action. It is a best practice to give each container its own sub-directory and to not re-name the Dockerfile.

Good to know, but you didn't create subdirectories either.

I don't think it's a good idea to have two identical Dockerfiles.

I agree. The two identical Dockerfiles are there for co-existence.

I will update the base container workflow and add a sub directory.

Moved the base Dockerfile to a new base directory. Removed the redundant Dockerfile.activator and updated the build service GitHub Action.

Dockerfile

kfindeisen · 2024-01-26T23:34:38Z

doc/playbook.rst

@@ -65,6 +65,29 @@ Any subsequent builds of the service container will build against both bases.

 This is the only situation in which a change to ``BASE_TAG_LIST`` should be committed to ``main``.

+Release Procedure
+-----------------


Should this be a subsection of "Containers" rather than another top-level section? It's true that the containers are what we're versioning, but if I were looking at the table of contents I wouldn't have thought to look here.

Moved Release instructions to a subsection of Containers. Thanks for suggestion!

??? My question was because it was a subsection of Containers to begin with...

kfindeisen · 2024-01-26T23:36:06Z

doc/playbook.rst

+Releases are largely automated through GitHub Actions (see the `ci-release.yaml <https://github.com/lsst-dm/prompt_processing/actions/workflows/ci-release.yaml>`  workflow file for details). When a semantic version tag is pushed to GitHub, Prompt Processing Docker images are published on GitHub and Docker Hub with that version.
+
+Regular releases happen from the ``main`` branch after changes have been merged.
+From the ``main`` branch you can release a new major version (``X.0.0``), a new minor version of the current major version (``X.Y.0``), or a new patch of the current major-minor version (``X.Y.Z``).  Release tags are semantic version identifiers following the :pep:`440` specification.


When I try following the PEP 440 link, it throws up a warning saying the document is no longer current. Should this be a link to https://packaging.python.org/en/latest/specifications/version-specifiers/ instead?

It's worth noting that the compatibility implications of semantic versioning are buried pretty far down the page (https://packaging.python.org/en/latest/specifications/version-specifiers/#compatible-release). But GitHub doesn't seem to have any documentation beyond the paragraph that shows up on the New Release page, and I assume you deliberately avoided linking to semver.org.

For the purpose of distinguishing between major and minor versions, does compatibility mean:

For Prompt Processing,

ability to read fanned-out nextVisit messages

schema compatibility of APDB inputs/outputs and Alert Stream outputs

any in-code assumptions about the version(s) of the central Butler repo, but no requirements regarding dataset types, pipeline contents, etc.

For fan-out,

ability to read incoming Summit messages

schema compatibility of fanned-out messages

The question of compatibility for fanned-out messages is a bit problematic because, so long as we're just dumping them into a Python dataclass, almost any change is incompatible: removing a field will give an error unless the client provides a default, as usual for schemas, but adding a field will give an error unconditionally.

Fixed link to pep. Also updated instructions to use X.Y.Z for releases without prepending v to be consistent with other release numbering schemes used by LSST.

kfindeisen · 2024-01-26T23:46:26Z

doc/playbook.rst

+2. Tag the release
+
+At the HEAD of the ``main`` branch, create and push a tag with the semantic version:
+
+.. code-block:: sh
+
+   git tag -s vX.Y.Z -m "vX.Y.Z"
+   git push --tags


I've confirmed on next_visit_fan_out that this step is not necessary -- publishing the release triggers the push:tags listeners.

kfindeisen · 2024-01-26T23:51:42Z

doc/playbook.rst

@@ -65,6 +65,29 @@ Any subsequent builds of the service container will build against both bases.



The "Stable Base Containers" section above this point assumes that the latest base is used for ongoing development, while production builds should use BASE_TAG_LIST if necessary.

If the responsibility for production builds is shifting from build-service to ci-release, then maybe we should do it the other way around and require that latest refer to a "known good" container (which is what we've done in practice instead of messing around with BASE_TAG_LIST). This would require changing automatic runs of build-base to not set the latest tag (in practice almost all latest bases are built manually, so this is mostly a safety net).

doc/playbook.rst

kfindeisen

Please restore the PROMPT_PROCESSING_DIR definition at the minimum; the service will not work if this is not an absolute path. I'm happy to help with testing.

I've made a few other suggestions as well (in particular, please squash all fixup-type commits before final merge).

kfindeisen · 2024-02-08T19:17:49Z

.github/workflows/build-base.yml

@@ -60,14 +60,14 @@ jobs:
          echo "Eups tag = $(< eups.tag)"
      - name: Build image
        # Context-free build
+        working-directory: base


The comment refers to the run: block, so the new line should go above it.

~~Or, as I understand it, this change means the build is no longer context-free and the comment should be removed?~~ Reversed in a fixup commit.

Dockerfile

kfindeisen · 2024-02-08T19:41:10Z

Dockerfile.activator

Would it be possible to move the deletion of Dockerfile.activator to "Add Dockerfile for activator", so that it's clear to anyone reviewing the history that it's a move/rename rather than a creation?

kfindeisen · 2024-02-08T19:49:45Z

doc/playbook.rst

-However, it may happen that the ``latest`` base is used for development while production runs should use an older build.
-If this comes up, edit ``.github/workflows/build-service.yml`` and append the desired base build to the ``BASE_TAG_LIST`` variable.
-Any subsequent builds of the service container will build against both bases.
+To build a release:


Ok, now I see what happened. No, I think releases should have their own heading (whether a top-level section, or a subsection like before). All the instructions for different things get jumbled together otherwise.

Moved release management to its own section.

doc/playbook.rst

kfindeisen · 2024-02-08T19:55:32Z

doc/playbook.rst

 Releases are largely automated through GitHub Actions (see the `ci-release.yaml <https://github.com/lsst-dm/prompt_processing/actions/workflows/ci-release.yaml>`  workflow file for details). When a semantic version tag is pushed to GitHub, Prompt Processing Docker images are published on GitHub and Docker Hub with that version.

-Regular releases happen from the ``main`` branch after changes have been merged.
-From the ``main`` branch you can release a new major version (``X.0.0``), a new minor version of the current major version (``X.Y.0``), or a new patch of the current major-minor version (``X.Y.Z``).  Release tags are semantic version identifiers following the :pep:`440` specification.
+Regular releases happen from the ``main`` branch after changes have been merged.  From the ``main`` branch you can release a new major version (``X.0.0``), a new minor version of the current major version (``X.Y.0``), or a new patch of the current major-minor version (``X.Y.Z``).  Release tags are semantic version identifiers following the `pep 440 <https://peps.python.org/pep-0440/>` specification.

 1. Create a Release

 On GitHub.com, navigate to the main page of the repository.  To the right of the list of files, click **Releases**.  At the top of the page, click **Draft a new release**.  Type a tag using semantic versioning described in the previous section.  The Target should be the main branch.  


Emphasize that the tag is without the "v"?

kfindeisen · 2024-02-08T19:56:53Z

doc/playbook.rst

@@ -86,9 +80,19 @@ At the HEAD of the ``main`` branch, create and push a tag with the semantic vers

 .. code-block:: sh

-   git tag -s vX.Y.Z -m "vX.Y.Z"
+   git tag -s X.Y.Z -m "X.Y.Z"


👍 for removing the "v", but I still say this step is unnecessary. (And, in the unlikely event that the HEAD of main moved since you created the release, you'd be altering the definition of an existing tag.)

.github/workflows/build-base.yml

kfindeisen · 2024-02-13T01:01:29Z

doc/playbook.rst


 Select **Generate Release Notes**.  This will generate a list of commit summaries and of submitters.   Add text as follows.
-* Add the specific motivation for the release(for example, including a specific feature, preparing for a specific observing run))
+* Any specific motivation for the release(for example, including a specific feature, preparing for a specific observing run))


Suggested change

* Any specific motivation for the release(for example, including a specific feature, preparing for a specific observing run))

* Any specific motivation for the release (for example, including a specific feature, preparing for a specific observing run))

…tests or labels added. Testing initial functionality.

…ons. The Release Management updates include fixes to GitHub Action syntax on the and playbook documentation. The Build Base Dockerfile was moved to a base directory and GitHub action updated. The Release Management GitHub action is using a composite GitHub action that does not support specifying a Dockerfile file name. The Build Service Dockerfile was updated to remove the .activator suffix to accomodate and the Build Service GitHub action updated.

…dated text for what to include in release management summary.

dspeck1 requested a review from kfindeisen January 25, 2024 20:44

kfindeisen changed the title ~~Release Management GitHub Action~~ DM-40418: Release Management GitHub Action Jan 26, 2024

kfindeisen mentioned this pull request Jan 26, 2024

DM-42677: Don't forward visits with instrument="" lsst-dm/next_visit_fan_out#9

Merged

kfindeisen requested changes Jan 27, 2024

View reviewed changes

kfindeisen approved these changes Jan 30, 2024

View reviewed changes

dspeck1 force-pushed the tickets/DM-40418 branch 2 times, most recently from 0c9161a to 8f6af3e Compare February 8, 2024 17:27

kfindeisen requested changes Feb 8, 2024

View reviewed changes

dspeck1 force-pushed the tickets/DM-40418 branch 3 times, most recently from 7cf4a08 to 2f9e726 Compare February 12, 2024 22:40

kfindeisen approved these changes Feb 13, 2024

View reviewed changes

dspeck1 force-pushed the tickets/DM-40418 branch from 3fa0772 to 6897594 Compare February 13, 2024 16:23

dspeck1 added 3 commits February 13, 2024 10:28

Add initial workflow for release tagging of docker images. No docker …

6e2c021

…tests or labels added. Testing initial functionality.

Seperated release management into its own section of the playbook. Up…

8a68c17

…dated text for what to include in release management summary.

dspeck1 force-pushed the tickets/DM-40418 branch from 6897594 to 8a68c17 Compare February 13, 2024 16:28

dspeck1 merged commit c11a2e0 into main Feb 13, 2024
7 checks passed

dspeck1 deleted the tickets/DM-40418 branch February 13, 2024 17:05

kfindeisen mentioned this pull request Feb 20, 2024

DM-42618: Add startTime to the nextVisit schema in prompt processing lsst-dm/next_visit_fan_out#10

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DM-40418: Release Management GitHub Action #115

DM-40418: Release Management GitHub Action #115

dspeck1 commented Jan 18, 2024 •

edited

Loading

kfindeisen left a comment

kfindeisen Jan 26, 2024

kfindeisen Jan 26, 2024 •

edited

Loading

kfindeisen Jan 26, 2024

dspeck1 Jan 29, 2024

kfindeisen Jan 29, 2024

dspeck1 Jan 29, 2024

dspeck1 Feb 8, 2024

kfindeisen Jan 26, 2024

dspeck1 Feb 8, 2024

kfindeisen Feb 8, 2024

kfindeisen Jan 26, 2024 •

edited

Loading

kfindeisen Jan 30, 2024 •

edited

Loading

dspeck1 Feb 8, 2024

kfindeisen Jan 26, 2024

kfindeisen Jan 26, 2024 •

edited

Loading

kfindeisen left a comment •

edited

Loading

kfindeisen Feb 8, 2024

kfindeisen Feb 8, 2024 •

edited

Loading

kfindeisen Feb 8, 2024

dspeck1 Feb 12, 2024

kfindeisen Feb 8, 2024

kfindeisen Feb 8, 2024

kfindeisen Feb 13, 2024

		@@ -65,6 +65,29 @@ Any subsequent builds of the service container will build against both bases.

	* Any specific motivation for the release(for example, including a specific feature, preparing for a specific observing run))
	* Any specific motivation for the release (for example, including a specific feature, preparing for a specific observing run))

DM-40418: Release Management GitHub Action #115

DM-40418: Release Management GitHub Action #115

Conversation

dspeck1 commented Jan 18, 2024 • edited Loading

kfindeisen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfindeisen Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfindeisen Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

kfindeisen Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfindeisen Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

kfindeisen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kfindeisen Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dspeck1 commented Jan 18, 2024 •

edited

Loading

kfindeisen Jan 26, 2024 •

edited

Loading

kfindeisen Jan 26, 2024 •

edited

Loading

kfindeisen Jan 30, 2024 •

edited

Loading

kfindeisen Jan 26, 2024 •

edited

Loading

kfindeisen left a comment •

edited

Loading

kfindeisen Feb 8, 2024 •

edited

Loading