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

fix: handle netbox migration to disk set in MB #642

Merged
merged 9 commits into from
Jan 9, 2025

Conversation

sjurtf
Copy link
Contributor

@sjurtf sjurtf commented Sep 27, 2024

Update to handle NetBox 4.1.0 and above.

NetBox 4.1.0 introduced multiple breaking changes.
https://github.com/netbox-community/netbox/releases/tag/v4.1.0

Disk Size

Size was changed from GB to MB.
A migration when NetBox is started is updating the stored disk value by multiplying with 1000.

This proposed change ensures that the provider is backwards compatible in terms of HCL but still can support NetBox 4.1.0 and above.

Option number two is to rename disk_size_gb to disk_size_mb and break existing HCL.
If that is preferred let me know.

VLAN Group

The min_vid and max_vid fields on the VLAN group model have been replaced with vid_ranges, an array of starting and ending VLAN ID pairs.

I've removed the depercated implementation. VLAN Group support is pending changes in the API client.

Users Password

Password must now have at least one numeral.

Rack type

A new model has been introduced to model Rack Types.
I've removed the old references to type. The new Rack Model support support is pending changes in the API client.

@sjurtf
Copy link
Contributor Author

sjurtf commented Oct 2, 2024

@fbreckle Hi

Unfortunately, the last remaining resource requires changes in the API client in order to complete. Are you planning on patching the client?

@fbreckle
Copy link
Collaborator

Yes, I will update the client. I'm currenlty releasing a last version for 4.0.x with 4.0.11 support.

Then I'll get to the necessary client changes later today or tomorrow. Sorry for the delay!

As for the disk size change: I think we should change the attribute name to size_mb and include a migration for the attribute. I have actually never implemented a schema migration myelf, but a similar thing was implemented in https://github.com/e-breuninger/terraform-provider-netbox/blob/master/netbox/resource_netbox_virtual_machine_migrate_v0.go

@fbreckle
Copy link
Collaborator

Sigh.

Am I mistaken or is this one of these situations where the netbox devs did not properly update the openapi spec?

I have this in the openapi spec of netbox 4.1.3:

            "VLANGroupRequest": {
                "type": "object",
                "description": "Adds support for custom fields and tags.",
                "properties": {
                    "name": {
                        "type": "string",
                        "minLength": 1,
                        "maxLength": 100
                    },
                    "slug": {
                        "type": "string",
                        "minLength": 1,
                        "maxLength": 100,
                        "pattern": "^[-a-zA-Z0-9_]+$"
                    },
                    "scope_type": {
                        "type": "string",
                        "nullable": true
                    },
                    "scope_id": {
                        "type": "integer",
                        "nullable": true
                    },
                    "description": {
                        "type": "string",
                        "maxLength": 200
                    },
                    "tags": {
                        "type": "array",
                        "items": {
                            "$ref": "#/components/schemas/NestedTagRequest"
                        }
                    },
                    "custom_fields": {
                        "type": "object",
                        "additionalProperties": {}
                    }
                },
                "required": [
                    "name",
                    "slug"
                ]
            },

and I see this in the debug console when changing the new "vlan ids" field:

Content-Disposition: form-data; name="vid_ranges"

1-43

... but a vid_ranges field is nowhere to be found in the api spec. Not in 4.1.0, either. I guess I have to fiddle around and see what type it is myself. Just venting...

@fbreckle
Copy link
Collaborator

netbox-community/netbox#17488

@fbreckle fbreckle force-pushed the master branch 4 times, most recently from e43c4c4 to 35ff227 Compare January 7, 2025 14:02
@fbreckle
Copy link
Collaborator

fbreckle commented Jan 7, 2025

Ok tests are green and only disk size and its migration are to do. Then this can go live.

@fbreckle fbreckle marked this pull request as ready for review January 9, 2025 16:20
@fbreckle fbreckle merged commit 63ec2da into e-breuninger:master Jan 9, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants