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

Unable to add Proxmox chassis with token #2327

Closed
webteam-app opened this issue Mar 5, 2021 · 6 comments · Fixed by #2352
Closed

Unable to add Proxmox chassis with token #2327

webteam-app opened this issue Mar 5, 2021 · 6 comments · Fixed by #2352

Comments

@webteam-app
Copy link

Bug originally filed by ltrager at https://bugs.launchpad.net/bugs/1917953

I recently added Proxmox support to MAAS for a customer. Support has been backported to MAAS 2.9.2. Proxmox allows you to authenticate with either a password or a token. Adding a Proxmox chassis with a password works in the UI. If you try to add a Proxmox chassis with a token the UI insists a password must be given.

The power driver correctly defines which fields are required. power_pass, power_token_name, and power_token_secret all have required=False. When adding an individual machine the UI does the right thing and allows either to be set. Its only the add chassis form which is insisting a password must be set.

This fix should be backported to 2.9.

@sparkiegeek
Copy link
Contributor

@ltrager
Copy link

ltrager commented Mar 8, 2021

One thing I forgot to mention. From the error message it looks like the UI simply isn't sending fields other than type, hostname, username, and password. Other power types may be effected by this as well.

@Caleb-Ellis
Copy link
Contributor

@ltrager Ah ok, I've figured out the problem and been reminded of a workaround we're doing on the UI-side to get add_chassis working at all.

Do you know why the add_chassis api uses different parameter names to those provided by the power type? For example, when adding a virsh chassis the power type lists power_pass as one of the fields, but the add_chassis api expects password and will error otherwise. I can see the same thing with proxmox now (power_token_name given as the field name, but add_chassis expects token_name, amongst others). Right now we have a hardcoded map to translate between the two, but this is obviously less than ideal since we'll need to update it whenever a chassis power type is added or changed.

I can add new entries to the map for the new proxmox field names as a hotfix and backport to 2.9, but I think the ideal solution would be to have the add_chassis api just use the same parameter names.

@ltrager
Copy link

ltrager commented Mar 10, 2021

Each power driver defines its own parameters in the power driver. We try to keep things consistent but there may be some edge cases where there are some differences. When you add a single machine you have to use the power parameters defined by the power driver you are using. power_ is typically prefixed because when you add a machine your also defining other options such as hostname and mac_addresses

When you add a chassis we use a consistent set of parameters. Each power driver which supports the add_chassis command is hard coded to map to the proper power parameters when adding the discovered machine. I suspect the power_ prefix was dropped here because add_chassis only supports power arguments.

@Caleb-Ellis
Copy link
Contributor

Caleb-Ellis commented Mar 11, 2021

Each power driver which supports the add_chassis command is hard coded to map to the proper power parameters when adding the discovered machine.

@ltrager Is it possible to expose this map to the UI? Otherwise we'll be managing it in both places.

Here's what we have in the UI at the moment, where if it's not in the map the value is the same as what's defined in the power driver:

const chassisParameterMap = new Map([
  ["power_address", "hostname"],
  ["power_pass", "password"],
  ["power_port", "port"],
  ["power_protocol", "protocol"],
  ["power_token_name", "token_name"],
  ["power_token_secret", "token_secret"],
  ["power_user", "username"],
  ["power_verify_ssl", "verify_ssl"],
]);

@ltrager
Copy link

ltrager commented Mar 11, 2021

Technically there is an API description but its not formatted, its literally the comment of the add_chassis function.

I would use the map and if the key isn't found fall back to just removing the power_ prefix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants