Skip to content

Release v0.5.0 #353

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

Merged
merged 6 commits into from
Jun 15, 2018
Merged

Release v0.5.0 #353

merged 6 commits into from
Jun 15, 2018

Conversation

acatangiu
Copy link
Contributor

No description provided.

@acatangiu acatangiu requested a review from a team June 15, 2018 11:11
@acatangiu acatangiu changed the title Housekeeping for API YAML Housekeeping for API YAML (v0.5.0) Jun 15, 2018
Defines a token bucket with a fixed size/capacity and refill_time.
The refill-rate is derived from size and refill_time, and it is the constant
rate at which the tokens replenish.
Rate of consumption from the token bucket is unlimited which allows for bursts
Copy link
Member

Choose a reason for hiding this comment

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

I think there is a dash missing here before but.

@@ -1,10 +1,10 @@
swagger: "2.0"
info:
title: Firecracker v0.4 API
description: Firecraker v0.4 - RESTful public-facing API.
title: Firecracker v0.5 API
Copy link
Member

Choose a reason for hiding this comment

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

This should be commit with Cargo.toml before setting the tag.

summary: Creates new network interface with ID specified by iface_id path parameter.
If network interface with specified ID already exists,
updates its state based on new input. Will fail if update is not possible.
summary: Creates or updates a network interface.
Copy link
Member

Choose a reason for hiding this comment

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

We have a bug here, we should remove the update of network interfaces till we actually have it correctly implemented.

Copy link

Choose a reason for hiding this comment

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

#354 tracks the update bug.

summary: Creates or updates a network interface.
description:
Creates new network interface with ID specified by iface_id path parameter.
If network interface with specified ID already exists,
Copy link
Member

Choose a reason for hiding this comment

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

We should also say here: update is not supported yet or something like that.

This value is read-only for the control-plane.
description:
The current detailed state of the Firecracker instance.
This value is read-only for the control-plane.
Copy link
Member

Choose a reason for hiding this comment

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

We should also say here that it contains only the instance state: one of Uninitialized, Starting, Running. I think we shouldn't mention Halting and Halted since this also doesn't work at the time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They do work, starting after halting doesn't work and we have an open issue for that, but by no means should we remove it from the spec :)

@acatangiu acatangiu dismissed andreeaflorescu’s stale review June 15, 2018 12:41

Comments have been addressed

summary: Creates or updates the boot source.
description:
Creates new boot source if one does not already exists, otherwise updates it.
Will fail if update is not possible.
Copy link

Choose a reason for hiding this comment

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

It would be worth mentioning that updates are only allowed before the guest boots.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't turn this into documentation, these are already too verbose for my taste. Ideally the API spec should not expose or describe underlying program logic.

summary: Creates or updates a drive.
description:
Creates new drive with ID specified by drive_id path parameter.
If a drive with the specified ID already exists, updates its state based on new input.
Copy link

Choose a reason for hiding this comment

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

This is a bit more complicated:

  • if a drive with the same ID exists and the PUT comes before guest boot, the state is updated based on the new input;
  • if the guest is running, the input is irrelevant as a rescan is triggered and the drive is updated with info it reads from the OS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above

there is no CPU Template.
summary: Gets the machine configuration of the VM.
description:
Get the machine configuration of the VM. When called before the PUT operation, it
Copy link

Choose a reason for hiding this comment

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

s/Get/Gets

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

summary: Gets the machine configuration of the VM.
description:
Get the machine configuration of the VM. When called before the PUT operation, it
will return the default values for the vCPU count (=1), memory size (=128 MiB),
Copy link

Choose a reason for hiding this comment

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

This line ends abruptly in a comma, is there something else to add that got accidentally deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed that unfortunate layout.

When Hyperthreading is enabled, the vCPU count has to be either 1 or an even number.
is disabled, there are no restrictions regarding the vCPU count.
If one of the parameters does not have a correct value, the update fails and no fields are updated.
summary: Updates the machine configuration of the VM.
Copy link

Choose a reason for hiding this comment

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

Please rephrase with the wording from the description: "Updates the Virtual Machine Configuration"

summary: Creates new network interface with ID specified by iface_id path parameter.
If network interface with specified ID already exists,
updates its state based on new input. Will fail if update is not possible.
summary: Creates or updates a network interface.
Copy link

Choose a reason for hiding this comment

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

#354 tracks the update bug.

alxiord
alxiord previously approved these changes Jun 15, 2018
The only supported boot source is LocalImage.
summary: Creates or updates the boot source.
description:
Creates new boot source if one does not already exists, otherwise updates it.
Copy link
Contributor

Choose a reason for hiding this comment

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

exist not exists.

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.

@@ -25,7 +25,7 @@ produces:
paths:
Copy link
Contributor

Choose a reason for hiding this comment

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

There is kind of a comment on line 335 # Timestamp generated by FC - output only timestamp: for the timestamp field. Can t we turn that comment into the value of a description field?

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.

dianpopa
dianpopa previously approved these changes Jun 15, 2018
@acatangiu acatangiu changed the title Housekeeping for API YAML (v0.5.0) Release v0.5.0 Jun 15, 2018
@acatangiu acatangiu dismissed stale reviews from andreeaflorescu and dianpopa via a377bd3 June 15, 2018 15:44
@alxiord alxiord merged commit 030570c into firecracker-microvm:master Jun 15, 2018
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.

5 participants