debezium/dbz#2101 Add pipeline deployment lifecycle with Ansible-based container management - #493
debezium/dbz#2101 Add pipeline deployment lifecycle with Ansible-based container management#493d1vyanshu-kumar wants to merge 5 commits into
Conversation
…d container management Signed-off-by: divyanshu_Kumar <154233802+d1vyanshu-kumar@users.noreply.github.com>
…format mapping, and offset JDBC URLs Signed-off-by: divyanshu_Kumar <154233802+d1vyanshu-kumar@users.noreply.github.com>
…ployment Signed-off-by: divyanshu_Kumar <154233802+d1vyanshu-kumar@users.noreply.github.com>
|
Hi @mfvitale, @kmos, could you take a look at this when you get a chance? One thing I wanted to mention, during our call we discussed using file-based offset/schema storage for Host mode, but after working through the implementation I went with the same JDBC-based approach ( It also kept things simpler on the implementation side since we reuse the existing That said, if you'd prefer the file-based approach instead, I'm happy to rework it, just let me know! |
We should go with file based offset mounting as a volume on the target host. |
I agree. The target host shouldn't change like a k8s pod or volume. I think it's fine for now |
| server_port integer not null, | ||
| deployment_status varchar(255) not null, | ||
| config_hash varchar(255) not null, | ||
| deployed_at timestamp(6) with time zone not null, | ||
| primary key (id) | ||
| ); | ||
|
|
||
| alter table if exists host_deployment | ||
| add constraint FK_host_deployment_pipeline | ||
| foreign key (pipeline_id) | ||
| references pipeline; | ||
| references pipeline | ||
| on delete cascade; | ||
|
|
||
| alter table if exists host_deployment | ||
| add constraint FK_host_deployment_host_status |
There was a problem hiding this comment.
You need to create another change file. You cannot modify an old one.
| * @return the selected host entity | ||
| * @throws io.debezium.DebeziumException if no hosts are available | ||
| */ | ||
| HostStatusEntity select(List<HostStatusEntity> readyHosts, LoadCounter loadCounter); |
There was a problem hiding this comment.
LoadCounter is specific for the round-robin load based implementation. Should not be in the interface
| * used in config drift detection. | ||
| */ | ||
| @ApplicationScoped | ||
| public class HostPipelineMapper { |
There was a problem hiding this comment.
I think that you can simply this a lot using the io.debezium.platform.environment.operator.PipelineMapper.map(pipelineFlat) and the call the asConfiguration on the DebeziumServer returned from the map method.
This is effectively what the operator does to create the config map with the application.properties
| String sshAlias = allocation.hostStatus().getSshAlias(); | ||
| int port = allocation.allocatedPort(); | ||
|
|
||
| String containerName = hostConfig.containerNamePrefix() + pipelineId; |
There was a problem hiding this comment.
We should use the pipeline the name ad we do for the k8s
| containerName, hostConfig.debeziumServerImage(), | ||
| port, mappedConfig.configHash()); | ||
|
|
||
| CommandResult mkdirResult = ansibleRunner.createDirectory(sshAlias, configDir); |
There was a problem hiding this comment.
I suppose that all commands below this point are Ansible related. Since we will replace this part with the host agent, I could see an interface with the start, stop, restart and in this PR the implementation is done via Ansible and then the other one that will call the API?
| * this class will be swapped for HTTP REST calls — all other logic | ||
| * (host selection, port allocation, status tracking) stays unchanged. | ||
| */ | ||
| @Dependent |
There was a problem hiding this comment.
Should this be @ApplicationScoped?
In CDI, for a @Dependent scoped bean, the container will create a new instance specifically for each observer method invocation, then destroy it afterward. The instance injected into HostEnvironmentController, the one holding the executor, is never notified when the shutdown event is observed.
| * @return the command result | ||
| */ | ||
| public CommandResult copyContent(String sshAlias, String content, String destPath) { | ||
| String copyArgs = "content='" + content + "' dest=" + destPath + " mode=0644"; |
There was a problem hiding this comment.
The content argument is being wrapped here in single quotes; however, if any user-provided config value, such as a password or other connector property, contains a single quote, the Ansible copy module argument parsing will break.
We should either add escaping logic to guard against this, use Ansible's stdin, or write to a temp file and use src= rather than content=.
There was a problem hiding this comment.
I've switched from content='...' to writing a local temp file and using src= instead. The config content never appears in the Ansible command string now, so single quotes (or any other characters) in user-provided values can't break argument parsing.
| # Must point to a Postgres reachable from the remote Docker container. | ||
| # Override via HOST_OFFSET_JDBC_URL env var when the VM IP changes. | ||
| connection: | ||
| url: ${HOST_OFFSET_JDBC_URL:jdbc:postgresql://192.168.252.23:5432/testdb?loggerLevel=OFF} |
There was a problem hiding this comment.
It looks like the IP here might be specific to your environment? Same for HOST_SCHEMA_HISTORY_JDBC_URL below.
There was a problem hiding this comment.
@Naros This part must be removed since we want to use file based offset on the host as default offset/schema storage
There was a problem hiding this comment.
| controller.undeploy(999L); | ||
|
|
||
| // Give the executor time to process | ||
| Thread.sleep(500); |
There was a problem hiding this comment.
Could we not use this instead?
assertThat(latch.await(ASYNC_WAIT_SECONDS, TimeUnit.SECONDS)).isTrue();
There was a problem hiding this comment.
Thanks! Replaced all Thread.sleep(500) calls with CountDownLatch.await(ASYNC_WAIT_SECONDS, TimeUnit.SECONDS) , same pattern used in HostProvisioningServiceTest. No more timing-dependent flakiness.
…cycle Signed-off-by: divyanshu_Kumar <154233802+d1vyanshu-kumar@users.noreply.github.com>
f24260c to
7e0fe33
Compare
|
Hey @mfvitale @Naros @kmos - pushed a commit addressing all the review feedback here is summary:
Also fixed a container permission issue that came up during live testing - the Debezium Server container couldn't write to the root-owned data directory. Per @mfvitale's suggestion, added --user As we discussed, I've opened a separate issue for the shared group provisioning improvement as a follow-up: dbz#2451 I have verified everything end-to-end on a live host server, and it is working smoothly. Please feel free to pull the latest changes and test it on your end, and let me know if anything else needs attention! |
There was a problem hiding this comment.
I have done a round of review, my suggestion is to take a look to the following article and internal links and apply a refactoring.
| * @param hostStatus the locked and selected host entity | ||
| * @param allocatedPort the unique port assigned for this deployment | ||
| */ | ||
| public record HostAllocation(HostStatusEntity hostStatus, int allocatedPort) { |
There was a problem hiding this comment.
move this record outside this class
| @Transactional(REQUIRES_NEW) | ||
| public Optional<HostDeploymentEntity> findByPipelineId(Long pipelineId) { | ||
| return em.createQuery( | ||
| "SELECT d FROM host_deployment d WHERE d.pipeline.id = :pipelineId", |
| @Transactional(REQUIRES_NEW) | ||
| public List<HostDeploymentEntity> findByStatus(DeploymentStatus status) { | ||
| return em.createQuery( | ||
| "SELECT d FROM host_deployment d WHERE d.deploymentStatus = :status", |
| * @return the persisted deployment entity | ||
| */ | ||
| @Transactional(REQUIRES_NEW) | ||
| public HostDeploymentEntity createDeployment(Long pipelineId, Long hostStatusId, |
There was a problem hiding this comment.
is it really necessary to return an HostDeploymentEntity? it could be void?
| */ | ||
| @Transactional(REQUIRES_NEW) | ||
| public HostDeploymentEntity createDeployment(Long pipelineId, Long hostStatusId, | ||
| String containerName, String imageVersion, |
There was a problem hiding this comment.
reduce the number of parameter creating a new record class. Try to follow the rule max 3 params
|
|
||
| if (!finished) { | ||
| process.destroyForcibly(); | ||
| String timeoutMessage = "Ansible ad-hoc command timed out after " |
| return new CommandResult.Failure(output); | ||
| } | ||
| catch (IOException e) { | ||
| logger.errorv(e, "Failed to start Ansible ad-hoc process"); |
There was a problem hiding this comment.
same as the previous comment related to stacktrace
| } | ||
|
|
||
| @Override | ||
| public HostStatusEntity select(List<HostStatusEntity> readyHosts) { |
There was a problem hiding this comment.
Why using the entity? You could use a domain class
| * @see AnsibleHostProvisioner | ||
| */ | ||
| @ApplicationScoped | ||
| public class AnsibleCommandRunner { |
There was a problem hiding this comment.
you could refactor using command pattern: https://refactoring.guru/design-patterns/command
| String configPath = configDir + PATH_SEPARATOR + CONFIG_FILE_NAME; | ||
| String dataDir = hostConfig.dataBasePath() + PATH_SEPARATOR + containerName; | ||
|
|
||
| // 1. Create config directory |
There was a problem hiding this comment.
Instead of using comment in the code, express your intentions: extract classes that describe the business
|
|
||
| -- Host-based pipeline deployment tables | ||
| -- Purely additive: no changes to existing tables, sequences, indexes, or constraints. | ||
|
|
||
| create sequence host_status_SEQ start with 1 increment by 50; | ||
|
|
||
| create sequence host_deployment_SEQ start with 1 increment by 50; |
There was a problem hiding this comment.
@d1vyanshu-kumar Existing file should not be modified. If you need to add column to an already existing table you just need to crate the alter DDL statement on a new migration file.
There was a problem hiding this comment.
@mfvitale ok, I see it now. I moved the host tables out to a new file, but deleting them from V3.6.0 still counts as modifying it, so that's why this popped up again. My bad, makes sense now.
I've reverted V3.6.0 and V3.7.0.2 back to their original state and added the deployed_at column + the ON DELETE CASCADE change through a new migration file instead, so neither of the existing files get touched. Let me know if that looks right. 🙂
…eployment Signed-off-by: divyanshu_Kumar <154233802+d1vyanshu-kumar@users.noreply.github.com>



Fixes debezium/dbz#2101
Description
Implements the full pipeline deployment lifecycle for host-mode environments using Ansible ad-hoc commands over SSH instead of the future Host Agent. This allows pipelines to be deployed, monitored, stopped, started, and undeployed on remote servers without blocking on the Host Agent implementation.
Changes
Pipeline Configuration Mapping
HostPipelineMapperto convert pipeline objects into flatapplication.propertiesformat with source, sink, offset storage, schema history, transforms, and signal/notification defaultsPipelineMapperdebezium.format.keyanddebezium.format.valuefromJsonConvertertoJsonto match Debezium Server's expected format enumDeployment Service & Host Selection
HostDeploymentServicewith transactional deployment creation, pessimistic-locked host selection, and port allocationDeployStrategyinterface withRoundRobinStrategy(least-loaded, deterministic tie-breaking by host ID)deployedAttimestamp toHostDeploymentEntityfor deployment age trackingfindReadyHosts()query for container cleanup when deployment records are removed by cascadePipeline Controller
HostPipelineControllerorchestrating deploy, stop, start, and undeploy operations viaAnsibleCommandRunnercopymodule; containers are managed using Ansibleshellmodule running Docker commandspipeline_idON DELETE CASCADEby falling back todocker rm -fon all ready hostsArc.container()in CDI request context activation to support plain JUnit test executionStatus Monitoring
HostDeploymentStatusPollerthat checks container health every 30 seconds viadocker inspectdeployedAttimestamp when transitioning back to DEPLOYING (restart scenario) to prevent false FAILED markingansible_become_timeout(60s) to prevent Ansiblebecomehangs on slow hostsAnsible Command Runner
AnsibleCommandRunneras a reusable wrapper for executing Ansible ad-hoc commands with structured result handling via sealedCommandResultinterfacePipeline Deletion
ON DELETE CASCADEtoFK_host_deployment_pipelineforeign key so deleting a pipeline from the UI automatically removes the associatedhost_deploymentrow without a constraint violation@OnDelete(action = CASCADE)onHostDeploymentEntity.pipelineto keep the JPA model consistent with the database schemaDev Profile Configuration
%devprofile so that Debezium Server containers on remote hosts connect to the correct PostgreSQL instance instead oflocalhostTesting
HostPipelineMapperTest— 17 unit tests covering source/sink/transform mapping, offset/schema history config, signal defaults ordering, format mapping, and SHA-256 hashingHostPipelineControllerTest— 17 unit tests for deploy/stop/start/undeploy orchestration, idempotent redeployment cleanup, and cascade undeploy fallbackHostDeploymentStatusPollerTest— 12 unit tests covering all state transitions, grace period logic, config drift detection, and become timeout configurationHostDeploymentServiceIT— 15 integration tests against real PostgreSQL for deployment CRUD, status transitions, deployedAt lifecycle, pessimistic locking, ready host queries, and cascade deletionAnsibleCommandRunnerTest— unit tests for command construction and result parsingRoundRobinStrategyTest— unit tests for least-loaded selection and tie-breakingPR Checklist
main