hellt / vrnetlab

Make VM-based Network OSes run in Containerlab
https://containerlab.dev
MIT License
129 stars 88 forks source link

vMX vrnetlab launch fails due to AttributeError in launch.py #192

Closed quackbox closed 6 months ago

quackbox commented 6 months ago

Following the build process for vMX images, upon launching a lab, the image is unexpectedly exited due to the following error:

2024-05-04 04:14:18,849: vrnetlab   DEBUG    Creating overlay disk image
2024-05-04 04:14:18,871: vrnetlab   DEBUG    Creating overlay disk image
Traceback (most recent call last):
  File "/launch.py", line 481, in <module>
    vr = VMX(
  File "/launch.py", line 353, in __init__
    VMX_vfpc(self.version, conn_mode=conn_mode),
  File "/launch.py", line 259, in __init__
    self.version = version
AttributeError: can't set attribute

Problematic code snippet:

class VMX_vfpc(vrnetlab.VM):
    def __init__(self, version, conn_mode):
        super(VMX_vfpc, self).__init__(None, None, disk_image="/vmx/vfpc.img", num=1)
        self.version = version
        self.num_nics = 96

This bug appears to be occurring due to the existence of the version getter property in the vrnetlab.VM super class, which the VMX_vfpc class inherits from and initialises:

...
    @property
    def version(self):
        """Read version number from VERSION environment variable

        The VERSION environment variable is set at build time using the value
        from the makefile. If the environment variable is not defined please add
        the variables in the Dockerfile (see csr)"""
        version = os.environ.get("VERSION")
        if version is not None:
            return version
        raise ValueError("The VERSION environment variable is not set")
quackbox commented 6 months ago

I have created a PR to merge my proposed fix, which simply renames the version variable in the VMX_vfpc class.

https://github.com/hellt/vrnetlab/pull/193

quackbox commented 6 months ago

At first I thought this was vMX specific, but it appears this bug was introduced in PR https://github.com/hellt/vrnetlab/pull/181, specifically commit https://github.com/hellt/vrnetlab/pull/181/commits/557fd702cfc47b92c1ef43f957dbd6165f7ab7b8 for the c8000v.

This may be causing unexpected issues with other images. From a quick look, looks like the VQFX_vcp class for vQFX images is also affected, and possibly the CSR_vm class for CSR images.

As this change was made for the c8000v image, would it be worth changing that specific implementation - or fixing other images on an as-needed basis? Not sure what the intention with this getter is.

hellt commented 6 months ago

resolved in #193