andreasscherbaum / ansible-lxc-ssh

Ansible connection plugin using ssh + lxc-attach
45 stars 22 forks source link

Use with proxmox CTs #56

Closed jbrubake closed 8 months ago

jbrubake commented 9 months ago

I hacked the plugin to manage Proxmox CT's using pct exec instead of lxc-attach. Would using lxc-attach be preferable or should I fork this and make a pct_ssh plugin?

Besides changing the underlying commands there were some quoting differences between the two.

andreasscherbaum commented 9 months ago

I mean, I only forked it because the original maintainer gave it up.

The name implies that it is using LXC. The two options here are to fork it and change the name, or change the name of this tool to make it use more tools.

jbrubake commented 9 months ago

The changes necessary to make the current plugin work with pct exec are actually very simple so I think the advantages and disadvantages are:

  1. Fork it

Advantages: keeps this plugin 'lxc-pure' Disadvantage: someone sees this plugin (like I did) and assumes it manages Proxmox containers

  1. Merge changes to prefer pct exec if it exists (and probably change the project name, although not necessarily)

Advantages: allows this plugin to manage both standard LXC containers and Proxmox LXC containers (which is what I assumed it would do) Disadvantages: this plugin does an extra thing

It's your project so I don't really mind but I thought it was helpful to at least lay out why adding a feature to this project might not be a bad idea.

Either way it is still managing LXC containers, it's just potentially doing so without using lxc commands directly.

andreasscherbaum commented 9 months ago

@jbrubake I get where you are coming from, and why this is seemingly only a small addition.

My problem is that Proxmox is not included in the "LXC" name.

Let me ask a different question: what's a good name going forward, if this included pct as well? Maybe we can compromise and I rename the repository.

jbrubake commented 9 months ago

I think lxc-ssh is still appropriate because pct is just a different method of managing LXC containers. If you are running LXC containers normally, you would use lxc* to manage them. If you are running LXC containers in Proxmox you use the pct tools to manage them.

If the plugin name is related to what you are connecting to, then I think the current name is appropriate. When I searched for a way to manage my Proxmox containers I found this plugin and just assumed it would work.

andreasscherbaum commented 8 months ago

I was thinking about his issue, and also asked a few people about their opinion. The majority connects "lxc" with how the container are managed, with LXC. One person also pointed out that Proxmox does not necessarily use LXC, but can run KVM containers as well.

I get where you are coming from, however given the feedback I got, I prefer a fork and a appropriate name for the forked project.

jbrubake commented 8 months ago

Proxmox containers are lxc. Proxmox VMs are kvm. So I don't think that would confuse anyone. Regardless, your position is reasonable. Thanks for closing the loop on this.

My fork is ansible-pct-ssh. Currently it provides all the function of your plugin but it checks to see if pct is available first. Eventually I will rip out the lxc stuff and only support pct.

andreasscherbaum commented 8 months ago

I added your fork in the README.