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

Refactor Ant deployment #9049

Merged
merged 3 commits into from
Nov 26, 2024
Merged

Conversation

mackdk
Copy link
Contributor

@mackdk mackdk commented Jul 11, 2024

What does this PR change?

This PR refactors the ant script for improving the deployment process. Instead of having separate multiple tasks for deploying inside or outside the container, this change introduces a new property deploy.mode which specify how the process is performed and can have the following values:

  • local, the deployment is done by copying the files in the same machine where the repository lives
  • remote, the deployment is done on the remote machine defined by deploy.host. The copying is done by opening an SSH tunnel to the machine and transferring the data
  • container, the deployment is performed to a containerized server using mgrctl. The container can be locally installed or remote depending by the backend used by mgrctl which can be explicitly set by using the property container.backend
  • remote-container, the deployment is done on the remote machine defined by deploy.host. After establishing an SSH connection, mgrctl is used to deploy on a containerized server. This mode allows to deploy to containerized server, without needing mgrctl installed

After setting the deploy.mode and the deploy.host if needed, just call the deploy task. The following tasks have been updated to reflect the logic change:

  • deploy
  • deploy-static-resources
  • deploy-salt-files
  • restart-tomcat
  • restart-taskomatic

Please note that, currently, this change is not backward compatible, as I removed the tasks *-container tasks and the deploy-local task. Backwards compatibility could be achieved though, by restoring the deleted tasks, setting the property accordingly then calling the new task . I could even add these task along side the others, to keep them both for a "grace" period to test it more throughlly.

Examples

  • Deploy through ssh on a remote traditional server:
ant -f manager-build.xml -Ddeploy.mode=remote -Ddeploy.host=suma43-server.remote.host deploy
  • Deploy on a containerized server using mgrctl (which needs to be installed):
ant -f manager-build.xml -Ddeploy.mode=container deploy
  • Deploy to a remote containerized server using mgrctl (no need to have mgrctl install on the development machine):
ant -f manager-build.xml -Ddeploy.mode=remote-container -Ddeploy.host=uyuni-server.dev.local deploy

GUI diff

No difference.

  • DONE

Documentation

  • No documentation needed, since it's a development only change but once merged, the wiki needs to be updated

  • DONE

Test coverage

  • No tests: no code change

  • DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

  • No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

  • Re-run test "changelog_test"
  • Re-run test "backend_unittests_pgsql"
  • Re-run test "java_pgsql_tests"
  • Re-run test "schema_migration_test_pgsql"
  • Re-run test "susemanager_unittests"
  • Re-run test "javascript_lint"
  • Re-run test "spacecmd_unittests"

Before you merge

Check How to branch and merge properly!

Copy link
Contributor

👋 Hello! Thanks for contributing to our project.
Acceptance tests will take some time (aprox. 1h), please be patient ☕
You can see the progress at the end of this page and at https://github.com/uyuni-project/uyuni/pull/9049/checks
Once tests finish, if they fail, you can check 👀 the cucumber report. See the link at the output of the action.
You can also check the artifacts section, which contains the logs at https://github.com/uyuni-project/uyuni/pull/9049/checks.

If you are unsure the failing tests are related to your code, you can check the "reference jobs". These are jobs that run on a scheduled time with code from master. If they fail for the same reason as your build, it means the tests or the infrastructure are broken. If they do not fail, but yours do, it means it is related to your code.

Reference tests:

KNOWN ISSUES

Sometimes the build can fail when pulling new jar files from download.opensuse.org . This is a known limitation. Given this happens rarely, when it does, all you need to do is rerun the test. Sorry for the inconvenience.

For more tips on troubleshooting, see the troubleshooting guide.

Happy hacking!
⚠️ You should not merge if acceptance tests fail to pass. ⚠️

@mackdk mackdk marked this pull request as ready for review July 11, 2024 15:28
@mackdk mackdk requested review from a team as code owners July 11, 2024 15:28
@mackdk mackdk requested review from cbbayburt, cbosdo, rjmateus, Etheryte and rjpmestre and removed request for a team July 11, 2024 15:28
Etheryte
Etheryte previously approved these changes Jul 12, 2024
Copy link
Member

@Etheryte Etheryte left a comment

Choose a reason for hiding this comment

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

Thanks a lot for taking the time to improve this, this is a great upgrade and both me and Richa will use this daily. I've tested all the configurations that I would like to use this in and it works great.

For context for others reviewers, this change set unlocks deploying to all sorts of remote hosts easily, including when you're running your containers in a VM. This simplifies our day-to-day workflow tremendously, so I hope we can get this merged as soon as the branch is unlocked.

rjmateus
rjmateus previously approved these changes Jul 12, 2024
Copy link
Member

@rjmateus rjmateus left a comment

Choose a reason for hiding this comment

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

LGTM. I have tested a few, and it looks great to me :)
thanks for this change

@Etheryte Etheryte mentioned this pull request Jul 29, 2024
12 tasks
@mackdk mackdk force-pushed the restructure-ant-deployment branch from 3ed3d5b to 67568af Compare July 31, 2024 09:06
@rjpmestre
Copy link
Contributor

rjpmestre commented Sep 12, 2024

I have testing this in a remote container (w/podman) setup. I like it way better. Specially for not having to deal with podman remote. Thanks!

@cbosdo
Copy link
Contributor

cbosdo commented Sep 13, 2024

yeah! podman remote looks cool on paper, but the SSH part of it is very fragile

rjpmestre
rjpmestre previously approved these changes Oct 2, 2024
aaannz
aaannz previously approved these changes Nov 25, 2024
@mackdk mackdk dismissed stale reviews from aaannz, rjpmestre, and rjmateus via 9b27af5 November 26, 2024 09:56
@mackdk
Copy link
Contributor Author

mackdk commented Nov 26, 2024

@cbbayburt Seems like the enhanced changelog validation ignores the no changelog needed flag.

@mackdk mackdk merged commit 3010d74 into uyuni-project:master Nov 26, 2024
21 of 24 checks passed
Copy link
Contributor

⚠️ Some changelog tests have failed. @mackdk, please review and fix the changelog issues with an additional PR.

@mackdk mackdk deleted the restructure-ant-deployment branch November 26, 2024 13:25
@cbbayburt
Copy link
Contributor

@cbbayburt Seems like the enhanced changelog validation ignores the no changelog needed flag.

Yes, I'll merge a few fixes tomorrow. Thanks for the heads up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants