fix confluent-schema-registry.service dependencies

[roccogerminario]

confluent-kafka.target is not found, should be ".service"
moreover added dependency upon confluent-zookeper.service

[ghost]

It looks like @roccogerminario hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

[OneCricketeer]

The registry does not necessarily require Zookeeper.

Through, transitively, it does via Kafka.

How does systemd handle transitive service dependency ordering?

[roccogerminario]

The problem is not just with the schema-registry but for all the other services as well: confluent-kafka-connect.service, confluent-kafka-rest.service, confluent-kafka.service and confluent-ksql.service.
There are no ".target" units, they should be named ".service". I don't know if systemd manages transitive dependencies: in my test installation I also added "Restart=on-failure" and "RestartSec=30" because I found that the service was started correctly but failed after a couple of minutes or so...

[OneCricketeer]

Hmm. In my testing using the Ansible Playbooks, the services install and start fine. Though, I do understand your point about the "typo" of target vs service

[roccogerminario]

I installed on "Centos 7" with "yum"...

[OneCricketeer]

That's the OS I tested using a VM cluster

https://github.com/cricket007/cp-ansible/blob/addVagrant/vagrant/README.md

[mageshn]

@edenhill can you take a look at this PR when you get a chance?

[edenhill]

I don't believe the zookeeper dependency should be needed, isn't the change to confluent-kafka.service sufficient?
Also note that systemd startup-sequencing is only taken into account on bootup, not when manually starting a single service.

[lpsinger]

This, and all of the other confluent systemd units, still have broken dependencies due to these typos. Confirmed on Debian.

[andrewegel]

There is no guarantee that CP services will be deployed onto the same system. Therefore this is a not-desireable change. If customers are doing co-located deployments of CP services and want systemd unit files "aware" then they need to modify the systemd unit files themselves with tools such as ansible or puppet.

"out of the box" its assumed that services are deployed are isolated.

I propose that this change be dropped.

[lpsinger]

I understand that, but this is still a typo. If you do not support colocated deployments, then a better way to do that would be to remove the declared systemd dependencies, not to have broken dependencies.

The default configuration files shipped with the Confluent packages for Debian/Ubuntu are definitely set up for colocated services on the same machine.

[andrewegel]

Suggested change

After=network.target confluent-zookeeper.service confluent-kafka.service

[andrewegel]

I understand that, but this is still a typo. If you do not support colocated deployments, then a better way to do that would be to remove the declared systemd dependencies, not to have broken dependencies.

The default configuration files shipped with the Confluent packages for Debian/Ubuntu are definitely set up for colocated services on the same machine.

Then we should be removing the dependencies altogether do you agree? (See my suggested edit) - All of this was set up before I joined, so I can't speak to the reason why it was .target and not .service

Also speaking as the release master, I won't be making this change within a released version of CP (So you still need to use puppet/ansible to set your desired state), but would make one for the next major version (7.1 currently), as it represents a behavior change.

[lpsinger]

I understand that, but this is still a typo. If you do not support colocated deployments, then a better way to do that would be to remove the declared systemd dependencies, not to have broken dependencies.
The default configuration files shipped with the Confluent packages for Debian/Ubuntu are definitely set up for colocated services on the same machine.

Then we should be removing the dependencies altogether do you agree? (See my suggested edit) - All of this was set up before I joined, so I can't speak to the reason why it was .target and not .service

No, I don't agree. The Confluent Platform on-premise installation instructions for Debian and Ubuntu walk through a single-node setup, and those instructions don't quite work due to the typos in the systemd units.

If you are doing a multi-node setup and you simply don't have systemd units installed for the After dependencies, then systemd will simply ignore the missing dependencies. The best solution would be to fix the typos. That would not interfere at all with multi-node setups, and it would make single-node setups work correctly.

[cla-assistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}