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

[FEATURE REQUEST] Rename ip_address parameter, refactor parameters used outside of API #9

Closed
mthibaut opened this issue May 11, 2023 · 1 comment · Fixed by #33
Closed
Labels
enhancement New feature or request

Comments

@mthibaut
Copy link
Contributor

Is your feature request related to a problem? Please describe.

  • The ip_address profile parameter is used only to connect to the newly created guest over SSH post-creation. Its naming is confusing and suggests that the IP address on the guest is set this way; it is not (see the net0,1,2 etc params).
  • Additionally the other SSH related parameters (username/password) are prefixed with ssh_

Describe the solution you'd like

  • All SSH parameters should be clearly marked as such, e.g. with an ssh_ prefix
  • All parameters used internally by salt should be clearly separated from those transmitted to the API, possibly in two separate trees instead of in a single unclear jungle:
    1. API parameters. Users should be guided towards the Proxmox API documentation to find out more. Currently any change in the API requires documentation changes in salt, this is not what we want
    2. Non-API parameters. Users should be told by the salt documentation what parameters (such as SSH) can be configured

Perhaps eventually something like this? Not sure how other providers do this:

myprofile:
    provider: proxmox

    # Connect after install: the old ssh_username, ssh_password and ip_address parameters
    ssh:
        username: johndoe
        password: secret
        host: test.example.com 

    # Guest parameters
    guest:
        memory: 2048
        net0: virtio,ip=dhcp

Describe alternatives you've considered
Params in a single tree with clear names or separated in two trees

@I3urny
Copy link
Contributor

I3urny commented Sep 19, 2023

The ip_address is not only used for the ssh connection.
OpenVZ uses this as an optional parameter in the Proxmox API and LXC maps it to net0 parameter in the Proxmox API.
Take a closer look at the create_node() function:

# LXC specific network config
# OpenVZ allowed specifying IP and gateway. To ease migration from
# Proxmox 3, I've mapped the ip_address and gw to a generic net0 config.
# If you need more control, please use the net0 option directly.
# This also assumes a /24 subnet.
if "ip_address" in vm_ and "net0" not in vm_:
newnode["net0"] = "bridge=vmbr0,ip=" + vm_["ip_address"] + "/24,name=eth0,type=veth"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants