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

update vsphere machine config to use new networks endpoint #12263

Merged

Conversation

mantis-toboggan-md
Copy link
Member

@mantis-toboggan-md mantis-toboggan-md commented Oct 15, 2024

Summary

Fixes #9817
Fixes #12271

Occurred changes and/or fixed issues

This PR updates the vsphere machine config to use networks' moid instead of name. This requires the use a new endpoint for networks, meta/vsphere/networks-extended, which contains extra information.

Technical notes summary

This PR also corrects a reactivity issue in ArrayList:

Screen.Recording.2024-10-15.at.8.38.12.AM.mov

Areas or cases that should be tested

Create an RKE2 vsphere cluster and verify that:

  1. the machine pool is saved with the networks' moid instead of name
  2. you are able to select a network other than the default (first) option

Create an RKE2 vsphere cluster with at least 1 network and 'vapp' auto mode selected ('Use vapp to configure networks with network protocol profiles'), verify that the save request vapp properties reference the network name, not full path (and not moid), see #12271 (comment)

I only had 1 network in my test environment so I added fake ones in the loadNetworks method to verify that I could add/remove different networks:

    async loadNetworks() {
      set(this, 'networksResults', null);

      const options = await this.requestOptions('networks-extended', this.value.datacenter);
      const content = options.map((opt) => {
        return { label: `${ opt.name } (${ opt.moid })`, value: opt.moid };
      });

      content.push({ label: 'abc-123-name (moid-123-abc)', value: 'moid-123-abc' });
      content.push({ label: 'abc-123-name (moid-456-def)', value: 'moid-456-def' });

      this.resetValueIfNecessary('network', content, options, true);

      set(this, 'networksResults', content);
    }
    

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

@mantis-toboggan-md mantis-toboggan-md marked this pull request as ready for review October 16, 2024 18:52
Copy link
Member

@momesgin momesgin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mantis-toboggan-md mantis-toboggan-md merged commit d23db14 into rancher:master Oct 16, 2024
31 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
2 participants