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

VirtualMachine without a cluster set throws exception when viewed through API #2587

Closed
Grokzen opened this issue Nov 14, 2018 · 4 comments
Closed

Comments

@Grokzen
Copy link
Contributor

Grokzen commented Nov 14, 2018

Environment

  • Python version: 3.6.5
  • NetBox version: 2.4.6

Steps to Reproduce

  • Create a VirtualMachine object.
  • Do not set a cluster object on it, or set it to None
  • View the created VM in the API

Expected Behavior

API endpoint not to throw an exception

Observed Behavior

Following exception is returned to the user

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/django/contrib/staticfiles/handlers.py", line 66, in __call__
    return self.application(environ, start_response)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/wsgi.py", line 146, in __call__
    response = self.get_response(request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 81, in get_response
    response = self._middleware_chain(request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py", line 37, in inner
    response = response_for_exception(request, exc)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py", line 87, in response_for_exception
    response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py", line 122, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)
  File "/usr/local/lib/python3.6/site-packages/django_extensions/management/technical_response.py", line 37, in null_technical_500_response
    six.reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python3.6/site-packages/six.py", line 692, in reraise
    raise value.with_traceback(tb)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py", line 35, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 128, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 126, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.6/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/viewsets.py", line 103, in view
    return self.dispatch(request, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/views.py", line 483, in dispatch
    response = self.handle_exception(exc)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/views.py", line 443, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/views.py", line 480, in dispatch
    response = handler(request, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/mixins.py", line 58, in retrieve
    return Response(serializer.data)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/serializers.py", line 536, in data
    ret = super(Serializer, self).data
  File "/usr/local/lib/python3.6/site-packages/rest_framework/serializers.py", line 262, in data
    self._data = self.to_representation(self.instance)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/serializers.py", line 503, in to_representation
    ret[field.field_name] = field.to_representation(attribute)
  File "/usr/local/lib/python3.6/site-packages/rest_framework/fields.py", line 1855, in to_representation
    return method(value)
  File "/opt/netbox/netbox/virtualization/api/serializers.py", line 142, in get_config_context
    return obj.get_config_context()
  File "/opt/netbox/netbox/extras/models.py", line 766, in get_config_context
    for context in ConfigContext.objects.get_for_object(self):
  File "/usr/local/lib/python3.6/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/netbox/netbox/extras/querysets.py", line 20, in get_for_object
    region = getattr(obj.site, 'region', None)
  File "/opt/netbox/netbox/virtualization/models.py", line 513, in site
    return self.cluster.site

AttributeError: 'NoneType' object has no attribute 'site'

Safeguards for this should be added to when netbox tries to build the ConfigContext for the VM.

Current workaround

We created a UNDEFINED cluster object and pointed all VM:s that do not have an cluster defined yet to avoid this issue.

@jeremystretch
Copy link
Member

jeremystretch commented Nov 14, 2018

Do not set a cluster object on it, or set it to None

Cluster assignment is mandatory. I did uncover a minor bug (#2589) with the API serializer for virtual machines wherein the cluster field is not required, but attempting to create a VM without a cluster will still correctly yield an IntegrityError:

django.db.utils.IntegrityError: null value in column "cluster_id" violates not-null constraint
DETAIL:  Failing row contains (5, 2018-11-14, 2018-11-14 15:29:03.558478+00, test-vm1, null, null, null, , null, null, null, null, null, 1, null, null).

I can't imagine how this wouldn't happen unless you've manually altered the database schema.

@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Nov 14, 2018
@Grokzen
Copy link
Contributor Author

Grokzen commented Nov 14, 2018

Yeah it was an old change we had added to allow for None value to be set on the cluster field. Our use-case for it was that we have an external system that integrates with netbox api to create VM:s. But our problem was that the external system could not make the decision on what vmware cluster the VM would be located to. That would be done by the internal operations team that owns the netbox instance based on other parameters that we could not make into code. So we would create the VM, either set None or our custom UNDEFINED cluster object and then when our internal team had made their decision they would update to the correct cluster object and deploy it.

I guess we could rollback the null=True addition and use the UNDEFINED object instead to force the old behavior.

@Grokzen
Copy link
Contributor Author

Grokzen commented Nov 14, 2018

Unless the mentioned use-case scenario would change anything, and using a undefined cluster object is good enough, i guess this can be closed.

@jeremystretch
Copy link
Member

There was a lot of discussion around clusters back in #142. IIRC we agreed that cluster assignment would be mandatory because there is no direct relationship from VirtualMachine to Site. Using a pseudo-cluster like you mention is probably the best approach for partial provisioning.

@jeremystretch jeremystretch removed the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Nov 14, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jan 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants