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

firecracker: Add support for default VM configuration #1615

Merged
merged 1 commit into from May 2, 2019

Conversation

mcastelino
Copy link
Contributor

@mcastelino mcastelino commented May 2, 2019

firecracker: Add support for default VM configuration

Kata support specifing the default VM configuration via configuration.toml. This allows the system or cluster admin to choose the default (i.e minimum) size of the VM.

Add support in kata to respect the VM configuration for firecracker.

Fixes: #1594

Signed-off-by: Manohar Castelino manohar.r.castelino@intel.com

Copy link

@sboeuf sboeuf left a comment

Choose a reason for hiding this comment

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

Two nits, but the code looks good! Thanks @mcastelino for tackling this :)

@@ -351,6 +373,10 @@ func (fc *firecracker) startSandbox(timeout int) error {
return err
}

fc.fcSetVMBaseConfig(int64(fc.config.MemorySize),
Copy link

Choose a reason for hiding this comment

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

Please check the error returned here.

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

virtcontainers/fc.go Outdated Show resolved Hide resolved
Copy link
Contributor

@grahamwhaley grahamwhaley left a comment

Choose a reason for hiding this comment

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

lgtm, given @sboeuf feedback.
Do we need any docs updates to go with this, or is it 'obvious that this is how it should work' ?

@grahamwhaley
Copy link
Contributor

holding off a \ t e s t and merge until we hear back from @mcastelino

Kata support specifing the default VM configuration via
configuration.toml. This allows the system or cluster admin
to choose the default (i.e minimum) size of the VM.

Add support in kata to respect the VM configuration for firecracker.

Also refactor some code to make error handling uniform.

Fixes: kata-containers#1594

Signed-off-by: Manohar Castelino <manohar.r.castelino@intel.com>
@sboeuf
Copy link

sboeuf commented May 2, 2019

/test

@egernst egernst merged commit 2051dac into kata-containers:master May 2, 2019
@jcvenegas
Copy link
Member

Fedora job failed due to: kata-containers/tests#1449

• Failure [22.547 seconds]
state
/tmp/jenkins/workspace/kata-containers-runtime-fedora-PR/go/src/github.com/kata-containers/tests/functional/state_test.go:26
  container
  /tmp/jenkins/workspace/kata-containers-runtime-fedora-PR/go/src/github.com/kata-containers/tests/vendor/github.com/onsi/ginkgo/extensions/table/table.go:92
    with workload [true], timeWait 5 [It]
    /tmp/jenkins/workspace/kata-containers-runtime-fedora-PR/go/src/github.com/kata-containers/tests/vendor/github.com/onsi/ginkgo/extensions/table/table_entry.go:46

    Expected
        <int>: 1
    to equal
        <int>: 0

    /tmp/jenkins/workspace/kata-containers-runtime-fedora-PR/go/src/github.com/kata-containers/tests/functional/state_test.go:45
------------------------------
SSSSSSS

Summarizing 1 Failure:

[Fail] state container [It] with workload [true], timeWait 5 
/tmp/jenkins/workspace/kata-containers-runtime-fedora-PR/go/src/github.com/kata-containers/tests/functional/state_test.go:45

Ran 1 of 8 Specs in 22.548 seconds
FAIL! -- 0 Passed | 1 Failed | 0 Pending | 7 Skipped --- FAIL: TestFunctional (22.55s)

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.

firecracker: pass machine configuration
5 participants