Version notation fix - Githubissues

cisco-open / sdwan-lab-deployment-tool

This tool automates Cisco Catalyst SD-WAN lab deployment inside Cisco Modeling Labs (CML).

BSD 3-Clause "New" or "Revised" License

14 stars 2 forks source link

Version notation fix #11

Closed WojciechowskiPiotr closed 3 months ago

WojciechowskiPiotr commented 3 months ago

Description

If an image is already present in CML (i.e., already imported from replace ISO), it may have notation with hyphens instead of dots. This small patch fixes the handling of such notation in two places: 1) Validation of proper image for deployment 2) Listing of available images

It may be just a base to fix the notation handling in other parts of the code as well, I added support for hyphens in those two places because I don't have access to a fresh CML install to test image import via "setup" command here.

With this patch, I'll be able to deploy the lab with "sdwan-lab deploy 20.13.1" and "sdwan-lab deploy 20-13-1" commands.

Type of Change

[X] Bug Fix
[ ] New Feature
[ ] Breaking Change
[ ] Refactor
[ ] Documentation
[ ] Other (please describe)

tzarski0 commented 3 months ago

Hi @WojciechowskiPiotr, thanks for opening a PR. The dotted notation is used in many places around the script. Instead of covering two use cases every time (with dot and hyphen), how about we add the step in setup task to convert all the SD-WAN images with hyphen to dotted format?

WojciechowskiPiotr commented 3 months ago

I thought about that, but I wonder if it's possible. The image definitions are read-only, but even if this flag is removed, the ID cannot be changed, at least not from the GUI. It makes sense as image ID is referred in node definition. I think that needs further investigation.

I'm open to other solutions because if it's not changed, the only way would be to remove existing SDWAN images and set them up again using setup, which is not convenient either.

tzarski0 commented 3 months ago

@WojciechowskiPiotr I've implemented a converter in setup task on dev-2.0.10 branch. This should verify images and if there are any SD-WAN images using "-", it will convert them to ".". Could you please test it?

Make sure no SD-WAN labs are running on the server.
Install the tool from dev branch: pip install git+https://github.com/cisco-open/sdwan-lab-deployment-tool.git@dev-2.0.10
Run csdwan setup
Try deploying the lab

Let me know if that works!

WojciechowskiPiotr commented 3 months ago

Hi, It converted the image definition correctly. I'd like you to consider additional two cosmetic changes - put migration as a separate option (--convert-images) under setup and list available images at the end of setup (this also may be a separate option --list-images just to list available images) Thanks for implementing this change.

tzarski0 commented 3 months ago

Thank you! I will add option for listing images. As for conversion, I don't see a reason why we shouldn't convert it always. As original problem will be fixed in 2.0.10, I will close this PR now, thank you for your contribution!