Skip to content

Conversation

@manuelh-dev
Copy link

  • To enable dynamic deployment scenarios, only require a gateway when there is routes defined
  • To enable deployment of Windows VM (which do not use vsock), extend the CreateVirtualMachineSpec interface

Manuel Huber added 3 commits February 2, 2022 18:56
Use the endpoint name instead of id to ensure the resource path remains constant across VM reboots.
Otherwise, e.g. Windows VMs will recognize a different network adapter (leading to loss of static IP config)
@manuelh-dev manuelh-dev marked this pull request as ready for review March 11, 2022 18:41
@manuelh-dev manuelh-dev marked this pull request as draft March 11, 2022 18:41
@manuelh-dev manuelh-dev marked this pull request as ready for review March 11, 2022 18:43
@manuelh-dev manuelh-dev marked this pull request as draft March 11, 2022 18:43
@manuelh-dev
Copy link
Author

// Hot detach an endpoint from the compute system
request := hcsschema.ModifySettingRequest{
RequestType: requesttype.Remove,
ResourcePath: path.Join("VirtualMachine/Devices/NetworkAdapters", endpoint.Id),
Copy link

Choose a reason for hiding this comment

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

what's the original motivation to change it from Id to Name?

Copy link
Author

Choose a reason for hiding this comment

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

If we use the ID, the ID changes every time the VM is rebooted. This is a problem for the Windows VM because it detects a different physical network adapter and loses its networking configuration.

I.e. if we use the ID, a different physical network adapter is mapped into the VM each time the VM restarts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add an optional input parameter to allow caller specify resource name, if caller specify resource name, use it, otherwise use ID to ensure no conflict to existing resource name.

Same for hotAttachEndpoint

// Ensure the VM has access, we use opts.Id to create VM
if err := wclayer.GrantVmAccess(opts.Id, opts.VhdPath); err != nil {
return nil, err
return nil, fmt.Errorf("Failed to grant VM access to VHD file, error: %s", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

VHD file

use opts.VhdPath

}
if err := wclayer.GrantVmAccess(opts.Id, opts.IsoPath); err != nil {
return nil, err
return nil, fmt.Errorf("Failed to grant VM access to ISO file, error: %s", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ISO file

use opts.IsoPath

@johnsonshih
Copy link
Collaborator

}

This might not necessary when detaching, remove it if it's not required


Refers to: vm.go:371 in f60ece1. [](commit_id = f60ece1, deletion_comment = False)

for _, subnet := range ipam.Subnets {
if subnet.IpAddressPrefix != "" {
hasDefault := false
needsDefault := false
Copy link
Collaborator

Choose a reason for hiding this comment

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

false

decide the value of needsDefault by checking if subnet.Routes is empty before the loop, no need to set it in the loop.

johnsonshih added a commit that referenced this pull request Apr 1, 2022
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.

3 participants