Skip to content
This repository has been archived by the owner on Jul 28, 2021. It is now read-only.

Change the update container bridge call to use the modify call instead #391

Merged

Conversation

katiewasnothere
Copy link
Contributor

This PR updates OpenGCS to reuse the modify bridge call when issuing a request to update a container. Since the modify call was previously only used if the requested containerID is that of the UVM's pause container (null container ID), this PR additionally updates the code path to allow requests with specific container ID's to issue update calls but prevents them from issuing any host specific settings changes (such as adding a new mapped device in or a VPCI device).

Signed-off-by: Kathryn Baldauf kabaldau@microsoft.com

@katiewasnothere katiewasnothere requested a review from a team as a code owner January 26, 2021 02:22
@katiewasnothere
Copy link
Contributor Author

@kevpar FYI :)


func (c *Container) modifyContainerConstraints(ctx context.Context, rt prot.ModifyRequestType, cc *prot.ContainerConstraintsV2) (err error) {
if c.id == "" {
return errors.New("invalid parameter, containerID cannot be empty")
Copy link
Contributor

Choose a reason for hiding this comment

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

can this actually happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll remove :)

anmaxvl
anmaxvl previously approved these changes Jan 26, 2021
Copy link
Contributor

@anmaxvl anmaxvl left a comment

Choose a reason for hiding this comment

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

LGTM

default:
return errors.Errorf("the ResourceType \"%s\" is not supported", settings.ResourceType)
return errors.Errorf("the ResourceType \"%s\" is not supported for containers", settings.ResourceType)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use %q and get rid of the quotes

dcantah
dcantah previously approved these changes Feb 3, 2021
anmaxvl
anmaxvl previously approved these changes Feb 4, 2021
Signed-off-by: Kathryn Baldauf <kabaldau@microsoft.com>
@katiewasnothere
Copy link
Contributor Author

@dcantah and/or @anmaxvl would you PTAL when you get the chance? :) Rebased to account for the recent V1 removal

@katiewasnothere katiewasnothere merged commit 1e3104e into microsoft:master Feb 5, 2021
@katiewasnothere katiewasnothere deleted the update_to_modify_call branch February 5, 2021 19:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants