feat: add release markers #170

rubenruizdegauna · 2025-02-26T09:07:35Z

This PR will add release information to a file in the bucket where the package is being released.

After the lock is executed, an entry with start time will be appended to repo_path_root_dir/releases.json.
Just before the lock is released the entry will be loaded, end time will be set, and file will be persisted.

Example of data persisted:

[
  {
    "app_name": "newrelic-infra",
    "tag": "1.7.0",
    "run_id": "13549016185",
    "start": "2025-02-26T16:57:33Z",
    "end": "2025-02-26T16:58:47Z",
    "repo_name": "newrelic/infrastructure-agent",
    "schema": "custom",
    "schema_url": "https://raw.githubusercontent.com/newrelic/infrastructure-agent/test_publish_action_markers/build/upload-schema-linux-deb.yml"
  },
  {
    "app_name": "newrelic-infra",
    "tag": "1.7.0",
    "run_id": "13549016185",
    "start": "2025-02-26T16:58:53Z",
    "end": "2025-02-26T17:00:55Z",
    "repo_name": "newrelic/infrastructure-agent",
    "schema": "custom",
    "schema_url": "https://raw.githubusercontent.com/newrelic/infrastructure-agent/test_publish_action_markers/build/upload-schema-linux-rpm.yml"
  },
  {
    "app_name": "newrelic-infra",
    "tag": "1.7.0",
    "run_id": "13549016185",
    "start": "2025-02-26T17:01:01Z",
    "end": "2025-02-26T17:01:12Z",
    "repo_name": "newrelic/infrastructure-agent",
    "schema": "custom",
    "schema_url": "https://raw.githubusercontent.com/newrelic/infrastructure-agent/test_publish_action_markers/build/upload-schema-linux-targz.yml"
  }
]

Caveats: One release can call multiple times to the publish action. For example, the Infra Agent calls it once per package type (deb, rpm, tar.gz...). So multiple entries will be present for a release.

Ideally we would have just one, but then we need to move the responsibility to the user of the action.

This can be the first approach and we can discuss improvements.

rubenruizdegauna · 2025-02-27T07:31:13Z

publisher/config/config.go

+	SchemaURL            string
+	Schema               string


Added to the config so we can show this information in the release markersr.

Since they were already provided in the action, no change is needed in the inputs, right?

Right. They were used only to process them, download, and get the local path of the schema.

sigilioso

Thanks for addressing this! It will definitely help troubleshooting 🎉

I've left some comments for us to discuss 🙂

I think we should add some docs regarding the markers somewhere, could we use the README?

publisher/release/marker.go

sigilioso · 2025-02-27T08:10:59Z

publisher/release/marker_aws.go

I wonder if we should re-try por particular errors. Eg: error reading the markers.

I would leave it as an improvement in a separate PR. WDYT?

sigilioso · 2025-02-27T08:13:04Z

publisher/release/marker_aws.go

+	objOutput, err := s.client.GetObject(&s3.GetObjectInput{
+		Bucket: aws.String(s.conf.Bucket),
+		Key:    aws.String(s.markerPath()),
+	})
+	if err != nil {
+		return nil, fmt.Errorf("cannot read marker file: %w", err)
+	}


Is the markers file supposed to exist or will the action create it the first time?

It will be created if it doesn't exist https://github.com/newrelic/infrastructure-publish-action/pull/170/files#diff-746e45e2647cf17fb02b10957238994fabb26c6025dcb63eec2a0e14e7a55094R89

I totally missed that!

if err != nil { if !isNoSuchKeyError(err) { return Mark{}, err }

Shall we add a comment there to make more obvious?

👍 Added in 27048d7

sigilioso

LGTM

Although I'd love to have another pair of eyes here 👀

By the way, how are we enforcing its usage?

🤔 If anybody is using v1.X.Y (lower than latest or a commit) the release info will not appear in the file.

README.md

publisher/release/marker_aws.go

danielorihuela · 2025-03-03T12:08:47Z

publisher/release/marker_aws.go

+		return ErrLastMarkerEnded
+	}
+
+	if lastMarker.AppName != mark.AppName || !lastMarker.Start.Equals(mark.Start) {


Are packages uploaded 1 by 1 always? What if multiple teams try to release at the same time? This tool doesn't allow for it. Should this be allowed in the future?

There is a locking mechanism to prevent multiple actions running at the same time within the same S3 Bucket.

https://github.com/newrelic/infrastructure-publish-action/blob/main/publisher/lock/lock.go

rubenruizdegauna · 2025-03-03T13:41:09Z

LGTM

Although I'd love to have another pair of eyes here 👀

By the way, how are we enforcing its usage?

🤔 If anybody is using v1.X.Y (lower than latest or a commit) the release info will not appear in the file.
For now, we are only enforcing to v1 users. We need to think how to handle it (this is out of the scope of this PR, we need a Feature to work on the whole initiative)

publisher/release/marker_aws_test.go

danielorihuela · 2025-03-03T14:25:29Z

publisher/release/marker_aws_test.go

+		ReleaseInfo: ReleaseInfo{
+			AppName:   "my-app",
+			Tag:       "v1.2",
+			RunID:     "run3",
+			RepoName:  "repo3",
+			Schema:    "schema3",
+			SchemaURL: "url3",
+		},


(future) I see a lot of repetitions between tests. Maybe we could benefit from having some "basis" variables to reuse on the tests. Maybe is a terrible idea.

It is a good idea :) . We could use stubs with https://github.com/brianvoe/gofakeit for example

rubenruizdegauna force-pushed the feat/add_release_markers branch 7 times, most recently from b49b8d7 to 944b7c9 Compare February 26, 2025 16:21

rubenruizdegauna commented Feb 27, 2025

View reviewed changes

feat: add release markers

5e86214

rubenruizdegauna force-pushed the feat/add_release_markers branch from 944b7c9 to 5e86214 Compare February 27, 2025 07:33

rubenruizdegauna marked this pull request as ready for review February 27, 2025 07:34

rubenruizdegauna requested review from a team February 27, 2025 07:40

sigilioso reviewed Feb 27, 2025

View reviewed changes

sigilioso requested a review from a team February 27, 2025 11:53

release info

5dc1f53

rubenruizdegauna requested review from sigilioso and a team February 27, 2025 13:41

rubenruizdegauna force-pushed the feat/add_release_markers branch from e865340 to e2ac976 Compare February 28, 2025 14:03

add assertions for mocks expectations

1b5c84e

rubenruizdegauna force-pushed the feat/add_release_markers branch from e2ac976 to 1b5c84e Compare February 28, 2025 14:34

sigilioso previously approved these changes Mar 3, 2025

View reviewed changes

sigilioso requested review from a team March 3, 2025 09:37

add comment for isNoSuchKeyError

27048d7

rubenruizdegauna dismissed sigilioso’s stale review via 27048d7 March 3, 2025 10:14

danielorihuela reviewed Mar 3, 2025

View reviewed changes

publisher/release/marker_aws_test.go Show resolved Hide resolved

danielorihuela reviewed Mar 3, 2025

View reviewed changes

publisher/release/marker_aws_test.go Show resolved Hide resolved

danielorihuela reviewed Mar 3, 2025

View reviewed changes

new lines missing

fb041c3

rubenruizdegauna requested review from danielorihuela and sigilioso March 4, 2025 07:42

danielorihuela approved these changes Mar 4, 2025

View reviewed changes

rubenruizdegauna merged commit 4b7720c into main Mar 4, 2025
5 checks passed

rubenruizdegauna deleted the feat/add_release_markers branch March 4, 2025 13:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add release markers #170

feat: add release markers #170

rubenruizdegauna commented Feb 26, 2025 •

edited

Loading

rubenruizdegauna Feb 27, 2025

sigilioso Mar 3, 2025

rubenruizdegauna Mar 3, 2025

sigilioso left a comment

sigilioso Feb 27, 2025

rubenruizdegauna Feb 27, 2025

sigilioso Feb 27, 2025 •

edited

Loading

rubenruizdegauna Feb 27, 2025

sigilioso Mar 3, 2025

rubenruizdegauna Mar 3, 2025

sigilioso left a comment

danielorihuela Mar 3, 2025

rubenruizdegauna Mar 3, 2025

rubenruizdegauna commented Mar 3, 2025

danielorihuela Mar 3, 2025

rubenruizdegauna Mar 4, 2025

feat: add release markers #170

feat: add release markers #170

Conversation

rubenruizdegauna commented Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sigilioso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sigilioso Feb 27, 2025 • 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

sigilioso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rubenruizdegauna commented Mar 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rubenruizdegauna commented Feb 26, 2025 •

edited

Loading

sigilioso Feb 27, 2025 •

edited

Loading