-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fixes #15717 - Unable to assign a VM in Site to Cluster without Site #15763
base: develop
Are you sure you want to change the base?
Conversation
This PR has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further action is taken. |
@nopg can you please fix the merge conflicts |
10c1419
to
468a9b5
Compare
Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nopg I tried this PR -
I go to edit a VM that has a site assigned and I can't get the unassigned cluster in the dropdown (see screenshot below) I have created a 'tcluster' that doesn't appear in the selection. I think the queryset for the dropdown needs to be updated.
Note: without the PR I can assign a cluster (not assigned to a site) to a device that is assigned to a device fine, so I think this only applies to VM.
I swear i checked that myself and it was working (at least back on 3.x)! But yes I see that issue as well. I've got a lot going on the next few days but plan on updating this as soon as I can, just wanted to reply here first. |
9becb65
to
b816a5d
Compare
Ok @arthanson, done. As you mentioned this doesn't apply to Devices just VM's. I removed the query_params from VM's/Cluster, which is how it looks on the Device side as well. |
@nopg The dropdown works now but there is a validation error if you try to save |
@arthanson I can't reproduce that...I can save or create a VM-with-site into my Cluster without a site now, screenshot below. The validation should have been part of the initial change I made to virtualmachine.py in this PR. |
Fixes: #15717
Adds to existing IF statement for VirtualMachine.clean() (to match existing check for Device.clean()) to allow for VM to be assigned to a Cluster without a Site.
Updated existing VM test to check for this new use case, and added Device tests for Clusters with Sites matching/not matching as well.