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

Add VmEndpointRequest in hcn for eventual usage in the shim #1005

Merged
merged 1 commit into from
Apr 22, 2021

Conversation

dcantah
Copy link
Contributor

@dcantah dcantah commented Apr 21, 2021

HCS today for adding a network adapter to a virtual machine sets up the switch port itself by making a VmEndpointRequest. There are cases where we will want to do this ourselves, and we need the switch ID for this which will only exist after this request.

Below is an example of using this:

        portID, err := guid.NewV4()
	if err != nil {
		return errors.Wrap(err, "failed to generate guid for port")
	}

	vmEndpointRequest := hcn.VmEndpointRequest{
		PortId:           portID,
		VirtualNicName:   fmt.Sprintf("%s--%s", "cool-NIC-ID", portID),
		VirtualMachineId: guid.GUID{},
	}

	m, err := json.Marshal(vmEndpointRequest)
	if err != nil {
		return errors.Wrap(err, "failed to marshal endpoint request json")
	}

	if err := hcn.ModifyEndpointSettings(endpointID, &hcn.ModifyEndpointSettingRequest{
		ResourceType: hcn.EndpointResourceTypePort,
		RequestType:  hcn.RequestTypeAdd,
		Settings:     json.RawMessage(m),
	}); err != nil {
		return errors.Wrap(err, "failed to configure switch port")
	}

	// Get updated endpoint with new fields (need switch ID)
	ep, err := hcn.GetEndpointByID(endpointID)
	if err != nil {
		return errors.Wrapf(err, "failed to get endpoint %q", endpointID)
	}

	type ExtraInfo struct {
		Allocators []struct {
			SwitchId         string
			EndpointPortGuid string
		}
	}

	var exi ExtraInfo
	if err := json.Unmarshal(ep.Health.Extra.Resources, &exi); err != nil {
		return errors.Wrapf(err, "failed to unmarshal resource data from endpoint %q", endpointID)
	}

	if len(exi.Allocators) == 0 {
		return errors.New("no resource data found for endpoint")
	}

	// NIC should only ever belong to one switch but there are cases where there's more than one allocator
	// in the returned data. It seems they only ever contain empty strings so search for the first entry
	// that actually contains a switch ID and that has the matching port GUID we made earlier.
	var switchID string
	for _, allocator := range exi.Allocators {
		if allocator.SwitchId != "" && strings.ToLower(allocator.EndpointPortGuid) == portID.String() {
			switchID = allocator.SwitchId
			break
		}
	}

Signed-off-by: Daniel Canter dcanter@microsoft.com

HCS today for adding a network adapter to a virtual machine sets up the switch
port itself by making a VmEndpointRequest. There are cases where we will want to
do this ourselves, and we need the switch ID for this which will only exist after
this request.

Signed-off-by: Daniel Canter <dcanter@microsoft.com>
@dcantah
Copy link
Contributor Author

dcantah commented Apr 21, 2021

It isn't adding @microsoft/containerplat as a reviewer 😮

@dcantah dcantah requested a review from a team April 21, 2021 22:30
Comment on lines +97 to +98
Resources json.RawMessage `json:",omitempty"`
SharedContainers json.RawMessage `json:",omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious: why did you change these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Easier to unmarshal into something as it's just bytes instead of an interface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

var exi ExtraInfo
if err := json.Unmarshal(ep.Health.Extra.Resources, &exi); err != nil {
	return errors.Wrapf(err, "failed to unmarshal resource data from endpoint %q", endpointID)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, good point

Copy link
Contributor

@katiewasnothere katiewasnothere left a comment

Choose a reason for hiding this comment

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

LGTM

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