lyft/cni-ipvlan-vpc-k8s

Incorrect handling of errors in IPAM

Open

#39 opened on May 14, 2018

View on GitHub
 (1 comment) (0 reactions) (0 assignees)Go (57 forks)auto 404
buggood first issue

Repository metrics

Stars
 (365 stars)
PR merge metrics
 (PR metrics pending)

Description

The error handling in the IPAM add command is such that it is very hard to understand why something is failing. I've run into two problems that are due to the same general issue:

  1. The allocateClient doesn't differentiate between an interface already being maxed out on allowed IPv4 or IPv6 IP addresses and all IP addresses in a subnet being taken. Only the later is represented in the error here
  2. The error message from AllocateClient.AllocateIPFirstAvailableAtIndex(...) is clobbered unless there is more than one subnet that is tagged. This happens here.

For [2], if you have a single subnet for allocating Pod ENIs into, and the ENI is already attached to the host but has reached its maximum number of IPs, the error that will get returned is unable to create a new elastic network interface due to No subnets are available which haven't already been used but this is incorrect.

Contributor guide

Research direction
Examine the error handling logic in allocate.go and main.go, particularly the AllocateClient methods and the error message construction in the IPAM add command.
Tech stack
go
Domain
backendinfrastructure
Issue type
Bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
Active
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
Clear
Prerequisites
Go
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
75