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

feat: add version flag #125

Merged
merged 0 commits into from
Aug 15, 2023
Merged

feat: add version flag #125

merged 0 commits into from
Aug 15, 2023

Conversation

laurentsenta
Copy link
Contributor

@laurentsenta laurentsenta commented Aug 2, 2023

💣 depends on build-args in docker-container-action. Do not merge this is released and replaced in the current PR.

We want to output the version as a piece of metadata during testing. We will use this metadata during dashboard generation to signal gateways using different versions of the conformance test suite.

Example output: note the version in the dashboard
https://github.com/ipfs/gateway-conformance/actions/runs/5761510674/attempts/1#summary-15619660151

Supports #123

  • Introduce a version flag in the CLI
  • Update the workflows to release with the correct version flag
  • Output the version as a piece of metadata during test execution, if possible, related discussion in go project
  • Output version during dashboard generation

Note

This PR introduces metadata output, which we'll reuse for other pieces of data. This allows us to add more details to our tests, like a gateway checker id, specs id, etc.
For now, we're using a special log format --- META: .... This is a temporary workaround for the lack of metadata, which is being worked on in golang/go. We'll replace these with the official API when it's released.

@laurentsenta laurentsenta marked this pull request as draft August 2, 2023 16:01
@laurentsenta laurentsenta force-pushed the feat/introduce-version branch 18 times, most recently from 51050f0 to fb00a87 Compare August 3, 2023 14:06
@laurentsenta laurentsenta reopened this Aug 3, 2023
@laurentsenta
Copy link
Contributor Author

laurentsenta commented Aug 3, 2023

TIL: if you push the main branch to reset a branch, it'll close the associated PR

Dockerfile Outdated
Comment on lines 9 to 10
ARG VERSION=dev
RUN go build -ldflags="-X github.com/ipfs/gateway-conformance/tooling.Version=${VERSION}" -o ./gateway-conformance ./cmd/gateway-conformance
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about sourcing the metadata from git context? Similarly to what you already do in the Makefile. To me it feels more reliable - less moving parts.

Suggested change
ARG VERSION=dev
RUN go build -ldflags="-X github.com/ipfs/gateway-conformance/tooling.Version=${VERSION}" -o ./gateway-conformance ./cmd/gateway-conformance
RUN export TAG="$(git tag --points-at HEAD | sort -V | tail -n-1)" && \
export COMMIT="$(git rev-parse HEAD --short)" && \
export DIRTY_SUFFIX="$(test -n "`git status --porcelain`" && echo "-dirty" || true)" && \
export VERSION="${TAG:-$COMMIT$DIRTY_SUFFIX}" && \
go build -ldflags="-X github.com/ipfs/gateway-conformance/tooling.Version=${VERSION}" -o ./gateway-conformance ./cmd/gateway-conformance

For that to work, we'd only have to make sure the tags are fetched when we're building the docker image.

Also, one thing I noticed, we create tags slightly incorrectly. If you inspect v0, v0.3 and v0.3.0 now, you'll notice that v0 and v0.3 are on a different commit than v0.3.0. We should look into why that is - might need a fix in changelog based release action, I'll try to get to it today.

Copy link
Contributor

Choose a reason for hiding this comment

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

ipdxco/changelog-driven-release#9 <- I think this might be the fix for the tag mismatch. I'll test it later today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that'd create coupling / break single responsibility principle: instead of having a single argument, the docker build would depend on many side-effects to succeed (git binary, .git folder is around, repo is fetch'd as expected, etc).

@laurentsenta laurentsenta merged commit c3b6c39 into main Aug 15, 2023
14 of 16 checks passed
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.

2 participants