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 tool to install modules in lcow and plumb through shim #1195

Merged

Conversation

katiewasnothere
Copy link
Contributor

This PR adds support to install modules in LCOW. When drivers are specified on the pod or container config, the shim will make a request in the LCOW UVM to run the new tool install-drivers which creates a new mount location for the driver files and installs all driver modules using depmod and modprobe.

Signed-off-by: Kathryn Baldauf [email protected]

for _, driver := range args {
modules := []string{}

driverGUID, err := uuid.NewRandom()
Copy link
Contributor

@dcantah dcantah Oct 26, 2021

Choose a reason for hiding this comment

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

Is this new dep only because go-winio/pkg/guid doesn't build on Linux? 😔 microsoft/go-winio#169 We should move this forward, I'll approve and we should get a new release of go-winio out with this, didn't realize this (or know about that pr) until now. We shouldn't have to bring in a new dep for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok now that that's in, we'll just need to cut a release and revendor here

Copy link
Contributor

Choose a reason for hiding this comment

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

Did we come to a conclusion on cutting a new tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, but since this is an official google package I'd think the risk is probably minimal either way.

Copy link
Contributor

@dcantah dcantah Oct 28, 2021

Choose a reason for hiding this comment

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

Not worried about risk, more that we're introducing a new dep just to generate an ID when a project we maintain has the same functionality (if we cut a new tag) haha

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just do a follow up pr to remove the dep with a new tag of winio so we can move this forward

@dcantah
Copy link
Contributor

dcantah commented Oct 26, 2021

@katiewasnothere Can you push new commits when addressing feedback so it's easier to review on github? We can just squash at the end

@@ -61,3 +66,72 @@ func execPnPInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir stri
log.G(ctx).WithField("added drivers", driverDir).Debug("installed drivers")
return nil
}

func execModprobeInstallDriver(ctx context.Context, vm *uvm.UtilityVM, driverDir string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move LCOW install support routines out of pnp.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@msscotb msscotb 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.

3 participants