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

PoC to deploy with quadlets #255

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Mar 22, 2024

No description provided.

@ehelms ehelms marked this pull request as draft March 22, 2024 12:59
@ehelms ehelms force-pushed the quadlet-testing branch 2 times, most recently from ec8c8ec to da3d557 Compare March 26, 2024 01:49
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I noticed I had a review ready and then didn't submit it, but I see you already found the work that came from it (#256) and merged it.

lib/puppet/type/cpdb_create.rb Outdated Show resolved Hide resolved
manifests/service.pp Outdated Show resolved Hide resolved
manifests/service.pp Outdated Show resolved Hide resolved
manifests/install.pp Outdated Show resolved Hide resolved
@ehelms
Copy link
Member Author

ehelms commented Mar 26, 2024

@ekohl Have you seen examples of how we could test both deployment paths where a clean setup is needed for each? We would want to run through all acceptance tests, and then switch to container based and re-run all of them again.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

AFAIK you still need to ensure service is running/stopped. Perhaps we need to go a similar route as with others where you explicitly ensure tomcat is running/stopped and the same for the container service. We probably don't need to go as far for the packages to be absent too just yet.

lib/puppet/provider/cpdb_create/podman.rb Outdated Show resolved Hide resolved
lib/puppet/provider/cpdb_create/podman.rb Outdated Show resolved Hide resolved
Comment on lines 11 to 16
Volume=/etc/tomcat/logging.properties:/etc/tomcat/logging.properties
Volume=/etc/tomcat/server.xml:/etc/tomcat/server.xml
Volume=/etc/tomcat/login.config:/etc/tomcat/login.config
Volume=/etc/tomcat/cert-roles.properties:/etc/tomcat/cert-roles.properties
Volume=/etc/tomcat/cert-users.properties:/etc/tomcat/cert-users.properties
Volume=/etc/tomcat/conf.d/jaas.conf:/etc/tomcat/conf.d/jaas.conf
Volume=/etc/tomcat/tomcat.conf:/etc/tomcat/tomcat.conf
Copy link
Member

Choose a reason for hiding this comment

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

My reasoning behind using tomcat@ on Matrix was that we can use /etc/candlepin/tomcat or similar instead of /etc/tomcat. Then you can just mount that whole directory.

Perhaps we can start with using $tomcat_conf in here instead of hardcoding /etc/tomcat to make it portable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps we can start with using $tomcat_conf in here instead of hardcoding /etc/tomcat to make it portable

We can definitely move this to be a template using variables, that was a next step. First step was just getting it to pass tests (which it finally did).

My reasoning behind using tomcat@ on Matrix was that we can use /etc/candlepin/tomcat or similar instead of /etc/tomcat. Then you can just mount that whole directory.

I don't think I understand what the config directly has to do with the service name, but, honestly I would rather just the service be called candlepin and abstract away the tomcat part.

Copy link
Member

Choose a reason for hiding this comment

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

On EL 8+ you have tomcat.service which is a global since instance and tomcat@.service which uses /var/lib/tomcats/$NAME instead of /etc/tomcat. It's the multi-instance solution.

And I agree candlepin.service would be better. You can probably create a systemd drop in that adds the alias for you so you can use systemctl $action candlepin.

templates/tomcat/logging.properties Outdated Show resolved Hide resolved
@ehelms
Copy link
Member Author

ehelms commented Mar 26, 2024

I think one area worth noting is in the space of configuration. podman's quadlets has the ability to re-use some Kubernetes resources in order to bring cohesion between the deployments. For configuration, that's embracing ConfigMaps and defining configuration through this method (e.g. https://github.com/ygalblum/quadlet-demo/blob/main/quadlet-files/envoy-proxy-configmap.yml which can be read about via https://www.redhat.com/sysadmin/multi-container-application-podman-quadlet). I think that more Kube-native method would shift how we think about configuration.

@ekohl
Copy link
Member

ekohl commented Mar 26, 2024

I can imagine that we can still manage the config maps with Puppet. In theory the inputs that we want to tweak are the same, but the output format is different. That's what this module should provide: an abstraction where you can say "give me a candlepin that behaves in a certain way" and on a higher level (puppet-katello) we don't need to think about implementation details. Of course, that's theory.

@ehelms ehelms force-pushed the quadlet-testing branch 5 times, most recently from 990e6ed to 4973d52 Compare April 2, 2024 19:33
@ehelms ehelms force-pushed the quadlet-testing branch from 4973d52 to bf529ca Compare April 9, 2024 15:20
'image' => $candlepin::container_image,
}

file { '/etc/containers/systemd/tomcat.container':
Copy link
Member

Choose a reason for hiding this comment

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

I'd also be tempted to try southalc/podman#83. Comparing the two, your code doesn't take care of the daemon-reload that's needed when it changes. If we end up doing more with quadlets then it'll reduce the duplication.

@ehelms ehelms force-pushed the quadlet-testing branch 2 times, most recently from 97c62a6 to adb63eb Compare June 24, 2024 12:35
@ehelms ehelms force-pushed the quadlet-testing branch from adb63eb to 1a7530f Compare June 24, 2024 12:46
Comment on lines +19 to +27
Volume=/etc/tomcat/server.xml:/etc/tomcat/server.xml:Z
Volume=/etc/tomcat/login.config:/etc/tomcat/login.config:Z
Volume=/etc/tomcat/cert-roles.properties:/etc/tomcat/cert-roles.properties:Z
Volume=/etc/tomcat/cert-users.properties:/etc/tomcat/cert-users.properties:Z
Volume=/etc/tomcat/conf.d/jaas.conf:/etc/tomcat/conf.d/jaas.conf:Z
Volume=/etc/tomcat/tomcat.conf:/etc/tomcat/tomcat.conf:Z
Volume=/etc/candlepin:/etc/candlepin:Z
Volume=/var/log/candlepin:/var/log/candlepin:Z
Volume=/var/log/tomcat:/var/log/tomcat:Z
Copy link
Member

Choose a reason for hiding this comment

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

In my testing of a completely unrelated application I didn't need :Z, but I was using plain volumes. Is this really needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it depends on the application, the path, if it's read-only or read-write needed and SELinux policy as I understand it.

These two have some useful info:

Comment on lines +26 to +27
Volume=/var/log/candlepin:/var/log/candlepin:Z
Volume=/var/log/tomcat:/var/log/tomcat:Z
Copy link
Member

Choose a reason for hiding this comment

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

Can we configure candlepin (and Tomcat) to log everything to the journal and avoid these log directories?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair question - generally I think yes. There are some things already logged to the journal but not everything.

java_package => 'java-17-openjdk',
java_home => '/usr/lib/jvm/jre-17',
artemis_client_dn => Deferred('pick', ['', 'CN=ActiveMQ Artemis Deferred, OU=Artemis, O=ActiveMQ, L=AMQ, ST=AMQ, C=AMQ']),
use_container => #{container},
Copy link
Member

Choose a reason for hiding this comment

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

I'm not that happy with the duplication and essentially templating. For the container use case I assume the java_* parameters aren't needed, so perhaps just duplicate the whole file into both container.pp and package.pp files?

Copy link
Member Author

Choose a reason for hiding this comment

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

What duplication?

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.

2 participants