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

MDBF-832 & MDBF-828 - Prepare BBM deployment for Production #654

Merged
merged 10 commits into from
Nov 29, 2024

Conversation

RazvanLiviuVarzaru
Copy link
Collaborator

@RazvanLiviuVarzaru RazvanLiviuVarzaru commented Nov 27, 2024

Main Feature: MDBF-791 - Run Production BuildBot services in docker containers

In this pull request:

  • allow deployment of configuration files on new bbm PROD host
    • only manually on workflow dispatch event and only for Main branch
    • no service interaction. Only deploy configuration files, define Autogen/ masters and generate docker-compose file
  • create DEV_tags for master containers.
    • pushes to dev branch will create dev_ tags
    • NO move tags for now. We will update the Production tags manually until everything is stable enough.
    • make docker-compose be aware of dev_ tags in Development environment.
  • for bbm_deploy check step, validate both tags for each master to account for different BB versions (during upgrades for example). During BB upgrades our configuration changes (features/fixes) should be compatible with both versions unless there is a breaking change in BB upstream and we make that an exception (+close coordination between configuration changes and buildbot upgrade).
  • rename secrets for DEV and PROD for better classification.

Local validation with validate_master_cfg.sh

  • one should use -e DEV or -e PROD to choose between container images.

Pre-reqs:

  • @fauust Development secrets renaming
  • to provide secrets for new master production host once it's available.

For now, pushes in DEV will trigger the creation of a dev_ tag.
The workflow is not covering the replacement of the production tag because this should be a manual operation a few weeks/months after migration.

To be more precise, we should automate this flow after we bump buildbot version.
When master container is under development we must ensure  that configuration changes are valid on both buildbot master versions, PROD/DEV.
Let's say one will bring a new feature to Production independently of a master upgrade.
- no automation for now, only copy the configuration files, prepare docker-compose and Autogen/ masters
- will work only on manual dispatch from the Main branch
- renamed secrets for better env classification
@fauust
Copy link
Collaborator

fauust commented Nov 27, 2024

Good! I'll review more in details, but don't you think that you could avoid code duplication in the CI file by automatically defining the env DEV/PROD based on the branch?

@RazvanLiviuVarzaru
Copy link
Collaborator Author

RazvanLiviuVarzaru commented Nov 27, 2024

Good! I'll review more in details, but don't you think that you could avoid code duplication in the CI file by automatically defining the env DEV/PROD based on the branch?

Any example? In bbm_deploy.yml?
update: wait, I think I know what you mean, I'll come with an example soon.

@RazvanLiviuVarzaru
Copy link
Collaborator Author

Good! I'll review more in details, but don't you think that you could avoid code duplication in the CI file by automatically defining the env DEV/PROD based on the branch?

Any example? In bbm_deploy.yml? update: wait, I think I know what you mean, I'll come with an example soon.

@fauust This condition doesn't allow me to de-duplicate the code. If you know a better way...
github.event_name == 'workflow_dispatch' &&

@fauust
Copy link
Collaborator

fauust commented Nov 27, 2024

@fauust This condition doesn't allow me to de-duplicate the code. If you know a better way... github.event_name == 'workflow_dispatch' &&

I don't see how this could not be an env variable defined in a previous job... See: https://github.com/MariaDB/buildbot/blob/dev/.github/workflows/bbw_build_container_template.yml#L64-L77

@RazvanLiviuVarzaru
Copy link
Collaborator Author

@vladbogo
This is still needed in bbm_deploy?

# temporary fix of jade templating
          sed -i 's#https://ci.mariadb.org#https://ci.dev.mariadb.org#g' master-web/templates/home.jade

In Home.jade

| Packages built by buildbot can be downloaded from 
              if baseurl == "https://buildbot.mariadb.org/"
                a(href="https://ci.mariadb.org") here.
              else
                a(href="https://ci.dev.mariadb.org") here.

@vladbogo
Copy link
Collaborator

@RazvanLiviuVarzaru probably it needs a proper fix to work for both dev and prod. On prod, it should be removed if I remember correctly.

@fauust do you remember something else?

@fauust
Copy link
Collaborator

fauust commented Nov 28, 2024

From e51ca6a, I guess that it's needed.

@@ -10,6 +10,43 @@ err() {
exit 1
}

usage() {
echo "Usage: $0 -e <DEV|PROD>"
exit 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is technically not an error so exit code should be 0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Solved in, 9e553ca
Thanks!

@RazvanLiviuVarzaru
Copy link
Collaborator Author

From e51ca6a, I guess that it's needed.

@fauust / @vladbogo And what about this condition in Home.jade

| Packages built by buildbot can be downloaded from 
              if baseurl == "https://buildbot.mariadb.org/"
                a(href="https://ci.mariadb.org") here.
              else
                a(href="https://ci.dev.mariadb.org") here.

Use environment variables to distinguish between PROD and DEV environments.
Use format syntax to substitute the secret variable names for DEV and PROD.
The Set up environment variables step will mark the run as deployable if:

The repository is MariaDB/buildbot.
DEV-specific variables will be assigned if the branch is dev.
The deployment will go to PROD if the event is workflow_dispatch, with PROD-specific variables assigned when the branch is main.
The Start/Stop stack will only run for the Development environment.

The Home.jade replacement has been removed, as it should already be handled.
@RazvanLiviuVarzaru
Copy link
Collaborator Author

RazvanLiviuVarzaru commented Nov 28, 2024

@fauust This condition doesn't allow me to de-duplicate the code. If you know a better way... github.event_name == 'workflow_dispatch' &&

I don't see how this could not be an env variable defined in a previous job... See: https://github.com/MariaDB/buildbot/blob/dev/.github/workflows/bbw_build_container_template.yml#L64-L77

@fauust Please see my latest commit (and commit message) for code de-duplication: 9e553ca

I need to see how I can get rid of the one-liners so we have only 80 characters per line.
Code more readable here: 1038d4e

If tested this on a much simpler/dummy version of it, on my fork:

Non DEV/Main Branch Push: https://github.com/RazvanLiviuVarzaru/buildbot-r/actions/runs/12066479294
  Expected: do not run any deploy

PR to DEV: https://github.com/RazvanLiviuVarzaru/buildbot-r/actions/runs/12066506151/job/33647489581?pr=6
  Expected (OK): don't run deploy:

Push to DEV: https://github.com/RazvanLiviuVarzaru/buildbot-r/actions/runs/12066291842/job/33646821792
  Expected: run deploy on DEV
  Expansion OK for deploy: cd /srv/dev/docker-compose/ && ./generate-config.py --env=dev
  Expansion OK for secret name.

Push to Main: https://github.com/RazvanLiviuVarzaru/buildbot-r/actions/runs/12066423117
  Expected: don't run deploy

Manual dispatch of workflow on Main: https://github.com/RazvanLiviuVarzaru/buildbot-r/actions/runs/12066445087
  Expected: run deploy on PROD
  Expansion OK for deploy: cd /srv/prod/docker-compose/ && ./generate-config.py --env=prod
  Expansion OK for secret name
  Stop/Start Stack skipped: OK

@vladbogo
Copy link
Collaborator

From e51ca6a, I guess that it's needed.

@fauust / @vladbogo And what about this condition in Home.jade


| Packages built by buildbot can be downloaded from 

              if baseurl == "https://buildbot.mariadb.org/"

                a(href="https://ci.mariadb.org") here.

              else

                a(href="https://ci.dev.mariadb.org") here.

That already handles the case between dev and prod so I guess it should be fine and needed but probably open to improvement.

I guess that if you decide to use sed than aim for consistency and use sed for this type of check as well otherwise change the other to be consistent with this

@RazvanLiviuVarzaru
Copy link
Collaborator Author

RazvanLiviuVarzaru commented Nov 28, 2024

sed -i 's#https://ci.mariadb.org#https://ci.dev.mariadb.org#g' master-web/templates/home.jade

From e51ca6a, I guess that it's needed.

@fauust / @vladbogo And what about this condition in Home.jade


| Packages built by buildbot can be downloaded from 

              if baseurl == "https://buildbot.mariadb.org/"

                a(href="https://ci.mariadb.org") here.

              else

                a(href="https://ci.dev.mariadb.org") here.

That already handles the case between dev and prod so I guess it should be fine and needed but probably open to improvement.

I guess that if you decide to use sed than aim for consistency and use sed for this type of check as well otherwise change the other to be consistent with this

I mean,
sed -i 's#https://ci.mariadb.org#https://ci.dev.mariadb.org#g' master-web/templates/home.jade

Is replacing the values above. But this is already handled by if baseurl . So sed -i not needed, right?

@RazvanLiviuVarzaru
Copy link
Collaborator Author

RazvanLiviuVarzaru commented Nov 28, 2024

sed -i 's#https://ci.mariadb.org#https://ci.dev.mariadb.org#g' master-web/templates/home.jade

From e51ca6a, I guess that it's needed.

@fauust / @vladbogo And what about this condition in Home.jade


| Packages built by buildbot can be downloaded from 

              if baseurl == "https://buildbot.mariadb.org/"

                a(href="https://ci.mariadb.org") here.

              else

                a(href="https://ci.dev.mariadb.org") here.

That already handles the case between dev and prod so I guess it should be fine and needed but probably open to improvement.
I guess that if you decide to use sed than aim for consistency and use sed for this type of check as well otherwise change the other to be consistent with this

I mean, sed -i 's#https://ci.mariadb.org#https://ci.dev.mariadb.org#g' master-web/templates/home.jade

Is replacing the values above. But this is already handled by if baseurl . So sed -i not needed, right?

@vladbogo Look now on "Packages built by buildbot can be downloaded from"
https://buildbot.dev.mariadb.org/#/

I restored Home.jade as if the sed -i did not run at all.
The link to CI is OK.

@RazvanLiviuVarzaru RazvanLiviuVarzaru merged commit 9064394 into MariaDB:dev Nov 29, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants