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 reservations to admin control subnet allocs #369

Merged
merged 1 commit into from
Nov 17, 2015

Conversation

eyakubovich
Copy link
Contributor

This adds ability to add and remove reservations
for subnet allocations. A subnet is reserved with
the host IP (PublicIP). flannel will then use this
subnet for a host requesting a subnet.

Reservations are denoted in etcd as subnets with no
expiration (TTL is zero). When a reserved subnet is
allocated, it's TTL continues to be not set. If a
reservation is removed, the TTL is set to the usual
24 hours.

Also adds corresponding client/server APIs for
reservation management.

Fixes #280

@@ -241,6 +241,73 @@ func handleNetworks(ctx context.Context, sm subnet.Manager, w http.ResponseWrite
jsonResponse(w, http.StatusOK, wr)
}

// POST /{network}/reservations
func handleAddReservation(ctx context.Context, sm subnet.Manager, w http.ResponseWriter, r *http.Request) {
defer r.Body.Close()
Copy link

Choose a reason for hiding this comment

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

The handler does not need to close the request body. The http server will do that for you.

Refer: https://golang.org/pkg/net/http/#Request

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, never knew that.

@xiang90
Copy link

xiang90 commented Nov 17, 2015

Feel good about the overall approach.

t.Fatalf("failed to remove reservation: %v", err)
}

// The node should not have a TTL
Copy link

Choose a reason for hiding this comment

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

The node should have a TTL?

This adds ability to add and remove reservations
for subnet allocations. A subnet is reserved with
the host IP (PublicIP). flannel will then use this
subnet for a host requesting a subnet.

Reservations are denoted in etcd as subnets with no
expiration (TTL is zero). When a reserved subnet is
allocated, it's TTL continues to be not set. If a
reservation is removed, the TTL is set to the usual
24 hours.

Also adds corresponding client/server APIs for
reservation management.

Fixes flannel-io#280
@xiang90
Copy link

xiang90 commented Nov 17, 2015

Do we have an e2e test or integration test yet?

The code LGTM.

@eyakubovich
Copy link
Contributor Author

We don't have e2e tests. That's on a todo list and it's non-trivial task as it requires setting up etcd and multiple containers or VMs for flannel daemons.

eyakubovich added a commit that referenced this pull request Nov 17, 2015
Add reservations to admin control subnet allocs
@eyakubovich eyakubovich merged commit 0bf2638 into flannel-io:master Nov 17, 2015
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