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

Pr environment, open/updated pr workflow #1640

Closed
wants to merge 1 commit into from
Closed

Conversation

CDimonaco
Copy link
Member

Description

This pr aims to create the github workflow for creating and updating an ephimeral pull request environment.

How was this tested?

Act locally and manual triggering

@CDimonaco CDimonaco added the env Create an ephimeral environment for the pr branch label Jul 18, 2023
@CDimonaco CDimonaco self-assigned this Jul 18, 2023
@CDimonaco CDimonaco marked this pull request as ready for review July 18, 2023 10:57
@CDimonaco CDimonaco force-pushed the pr_env_open branch 5 times, most recently from 7977d2e to e113562 Compare July 18, 2023 11:22
@CDimonaco CDimonaco removed the env Create an ephimeral environment for the pr branch label Jul 18, 2023
@CDimonaco CDimonaco added the env Create an ephimeral environment for the pr branch label Jul 18, 2023
@CDimonaco CDimonaco force-pushed the pr_env_open branch 2 times, most recently from f6baf7d to 8fed068 Compare July 18, 2023 12:07
@CDimonaco CDimonaco marked this pull request as draft July 18, 2023 12:07
@CDimonaco CDimonaco removed the env Create an ephimeral environment for the pr branch label Jul 18, 2023
@CDimonaco
Copy link
Member Author

The pr is stale, waiting for ansible playbook to be updated.

Missing pieces

  • run the ansible playbook on the env machine
  • fire photofinish to the deployed machine after a fixed time (job already set)

@CDimonaco CDimonaco added the env Create an ephimeral environment for the pr branch label Jul 20, 2023
@CDimonaco CDimonaco force-pushed the pr_env_open branch 7 times, most recently from f165a1a to 550c962 Compare July 20, 2023 16:46
@CDimonaco
Copy link
Member Author

After dns issues are solved, add the photofinish step, we need to make sure that the docker image is always pulled and container restarted with the newest image for the provided tag

@CDimonaco CDimonaco force-pushed the pr_env_open branch 3 times, most recently from 74cd3a6 to fb06422 Compare July 25, 2023 14:51
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

It looks good in general.
I have just written down some nit-pick comments.
I will do other round when the PR is put as ready to be reviewed

- labeled

env:
MANTAINERS: "[\"cdimonaco\", \"dottorblaster\", \"fabriziosestito\", \"rtorrero\", \"nelsonkopliku\", \"arbulu89\",\"jagabomb\",\"emaksy\",\"jamie-suse\"]"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use some already predefined group instead of individual names?
We have the Developers groups for example.
Or Admins, or something like that.
Plain users don't look like the best option

env:
REGISTRY: ghcr.io
IMAGE_REPOSITORY: ghcr.io/${{ github.repository_owner }}/trento-web
IMAGE_TAG: ${{ github.event.pull_request.number }}-env
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we should put PR_NUMBER here, or remove the declaration above if we are not going to use it.


env:
MANTAINERS: "[\"cdimonaco\", \"dottorblaster\", \"fabriziosestito\", \"rtorrero\", \"nelsonkopliku\", \"arbulu89\",\"jagabomb\",\"emaksy\",\"jamie-suse\"]"
PR_ENV_LABEL: env
Copy link
Contributor

Choose a reason for hiding this comment

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

We would need to document this label usage

# cache-to: type=gha,mode=max

create_pr_environment:
name: Create or update the pr environment
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should put pr as capital letters (PR) in the texts?
To highlight that it is an acronym

needs: build-and-push-pr-image
env:
PR_BASE_URL: ${{ github.event.pull_request.number }}.prenv.trento.suse.com
PR_NUMBER: ${{ github.event.pull_request.number }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to re-declare this PR_NUMBER again?

server:
ansible_host: ${{ secrets.PR_ENV_MACHINE_IP }}
options: |
--extra-vars "web_postgres_db='${{ env.PR_NUMBER}}db' \
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's put an extra space after env.PR_NUMBER -> {{ env.PR_NUMBER }}

force_recreate_web_container='true' \
force_pull_images='true' \
trento_server_url='http://${{ env.PR_BASE_URL}}'"
run-photofinish-demo-env:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a new line before this job?

runs-on: ubuntu-20.04
needs: create_pr_environment
env:
TRENTO_PR_ENV_URL: "${{ github.event.pull_request.number }}.prenv.trento.suse.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

We could declare this on top of the file, to avoid re-declaring for each job

rabbitmq_vhost='${{ env.PR_NUMBER}}' \
rabbitmq_password='trento' \
prometheus_url='http://localhost' \
web_admin_password='adminpassword' \
Copy link
Contributor

Choose a reason for hiding this comment

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

We should put this web_admin_password in our secrets just in case

@CDimonaco CDimonaco force-pushed the pr_env_open branch 2 times, most recently from 4983189 to 7101d8c Compare August 7, 2023 11:07
@CDimonaco
Copy link
Member Author

CLOSED IN FAVOR OF #1704

@CDimonaco CDimonaco closed this Aug 10, 2023
@stefanotorresi stefanotorresi deleted the pr_env_open branch March 27, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
env Create an ephimeral environment for the pr branch
Development

Successfully merging this pull request may close these issues.

2 participants