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

Configure self-hosted to test the specified image #3

Merged
merged 4 commits into from
Jan 9, 2023

Conversation

chadwhitacre
Copy link
Member

@chadwhitacre chadwhitacre commented Jan 9, 2023

We weren't testing the image passed in via image_url.

Generalization of the fast fix in getsentry/sentry#42953.

action.yml Outdated Show resolved Hide resolved
@chadwhitacre
Copy link
Member Author

Validating in getsentry/sentry#42959.

@chadwhitacre
Copy link
Member Author

Here's a run ...

@chadwhitacre
Copy link
Member Author

I don't understand why it's not failing since image_var is not present (right?) but should be required.

@chadwhitacre
Copy link
Member Author

Yeah I don't get it. I get an empty string for the value of image_var when not provided. I would expect a validation error way earlier in the run.

  echo "=us.gcr.io/sentryio/sentry:0eba2828f044e750749a8daffdb6c58ac8bf98ca" > .env.custom

https://github.com/getsentry/sentry/actions/runs/3875880798/jobs/6609604022#step:5:483

@chadwhitacre
Copy link
Member Author

Let's depend on the project_name, and leave the status quo as-is for other vars.

Copy link
Member

@hubertdeng123 hubertdeng123 left a comment

Choose a reason for hiding this comment

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

Seems fine to me, although a bit messy. Too bad requiring the input doesn't seem to be working

@chadwhitacre
Copy link
Member Author

Looks good! 👍

SENTRY_IMAGE=us.gcr.io/sentryio/sentry:3adb839e6b8dcf90be7873100dfa3d9ce2de63dd

https://github.com/getsentry/sentry/actions/runs/3876881234/jobs/6611258501#step:4:496

@chadwhitacre chadwhitacre merged commit ade3e64 into main Jan 9, 2023
@chadwhitacre chadwhitacre deleted the cwlw/configure-for-test-image branch January 9, 2023 19:24
chadwhitacre added a commit to getsentry/sentry that referenced this pull request Jan 9, 2023
This is a follow-up to #42953
now that the fix has been generalized in the action in
getsentry/action-self-hosted-e2e-tests#3.
chadwhitacre added a commit to getsentry/relay that referenced this pull request Jan 10, 2023
@chadwhitacre
Copy link
Member Author

chadwhitacre commented Jan 10, 2023

Quick post-mortem/retro on this w/ @ethanhs ... lesson learned for next time: move to PR/review workflow off the bat in the action vs. direct-to-main in action (and counting on review from the PR in the other repo).

olksdr pushed a commit to getsentry/relay that referenced this pull request Jan 11, 2023
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