Skip to content

Add CNS IPAM check to not change state #614

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.

Sign up for GitHub

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 3 commits into from
Jul 15, 2020
Merged

Add CNS IPAM check to not change state #614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
66aa9ac
add check to cns to not overwrite state
matmerr Jul 15, 2020
e043161
use CreateOrUpdateNetworkContainer API instead of directly adding ipc…
matmerr Jul 15, 2020
6543c5c
rename test for clarity
matmerr Jul 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cns/NetworkContainerContract.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ type CreateNetworkContainerRequest struct {
LocalIPConfiguration IPConfiguration
OrchestratorContext json.RawMessage
IPConfiguration IPConfiguration
SecondaryIPConfigs map[string]ContainerIPConfigState //uuid is key
MultiTenancyInfo MultiTenancyInfo
CnetAddressSpace []IPSubnet // To setup SNAT (should include service endpoint vips).
Routes []Route
Expand Down
10 changes: 7 additions & 3 deletions cns/cnsclient/cnsclient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,14 @@ var (
func addTestStateToRestServer(svc *restserver.HTTPRestService) {
// set state as already allocated
state1, _ := restserver.NewPodStateWithOrchestratorContext(testIP1, 24, testPod1GUID, testNCID, cns.Available, testPod1Info)
ipconfigs := []*cns.ContainerIPConfigState{
state1,
ipconfigs := map[string]cns.ContainerIPConfigState{
state1.ID: state1,
}
svc.AddIPConfigsToState(ipconfigs)
nc := cns.CreateNetworkContainerRequest{
SecondaryIPConfigs: ipconfigs,
}

svc.CreateOrUpdateNetworkContainerWithSecondaryIPConfigs(nc)
}

func getIPConfigFromGetNetworkContainerResponse(resp *cns.GetIPConfigResponse) (net.IPNet, error) {
Expand Down
89 changes: 67 additions & 22 deletions cns/restserver/ipam.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,21 @@ func newIPConfig(ipAddress string, prefixLength uint8) cns.IPSubnet {
}
}

func NewPodState(ipaddress string, prefixLength uint8, id, ncid, state string) *cns.ContainerIPConfigState {
func NewPodState(ipaddress string, prefixLength uint8, id, ncid, state string) cns.ContainerIPConfigState {
ipconfig := newIPConfig(ipaddress, prefixLength)

return &cns.ContainerIPConfigState{
return cns.ContainerIPConfigState{
IPConfig: ipconfig,
ID: id,
NCID: ncid,
State: state,
}
}

func NewPodStateWithOrchestratorContext(ipaddress string, prefixLength uint8, id, ncid, state string, orchestratorContext cns.KubernetesPodInfo) (*cns.ContainerIPConfigState, error) {
func NewPodStateWithOrchestratorContext(ipaddress string, prefixLength uint8, id, ncid, state string, orchestratorContext cns.KubernetesPodInfo) (cns.ContainerIPConfigState, error) {
ipconfig := newIPConfig(ipaddress, prefixLength)
b, err := json.Marshal(orchestratorContext)
return &cns.ContainerIPConfigState{
return cns.ContainerIPConfigState{
IPConfig: ipconfig,
ID: id,
NCID: ncid,
Copy link
Member

@neaggarwMS neaggarwMS Jul 15, 2020

Choose a reason for hiding this comment

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

Who is going to call this API?

Copy link
Member

@neaggarwMS neaggarwMS Jul 15, 2020

Choose a reason for hiding this comment

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

can we rename it to just CreateorUpdateNetworkContainer?

Copy link
Member

@neaggarwMS neaggarwMS Jul 15, 2020

Choose a reason for hiding this comment

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

As discussed offline, this will require more changes to handle create and update. Lets do that change in a separate pr.

Expand All @@ -47,7 +47,7 @@ func (service *HTTPRestService) requestIPConfigHandler(w http.ResponseWriter, r
var (
err error
ipconfigRequest cns.GetIPConfigRequest
ipState *cns.ContainerIPConfigState
ipState cns.ContainerIPConfigState
returnCode int
returnMessage string
)
Expand Down Expand Up @@ -122,7 +122,7 @@ func (service *HTTPRestService) releaseIPConfigHandler(w http.ResponseWriter, r
return
}

func validateIPConfig(ipconfig *cns.ContainerIPConfigState) error {
func validateIPConfig(ipconfig cns.ContainerIPConfigState) error {
if ipconfig.ID == "" {
return fmt.Errorf("Failed to add IPConfig to state: %+v, empty ID", ipconfig)
}
Expand All @@ -138,33 +138,52 @@ func validateIPConfig(ipconfig *cns.ContainerIPConfigState) error {
return nil
}

func (service *HTTPRestService) CreateOrUpdateNetworkContainerWithSecondaryIPConfigs(nc cns.CreateNetworkContainerRequest) error {
return service.addIPConfigsToState(nc.SecondaryIPConfigs)
}

//AddIPConfigsToState takes a lock on the service object, and will add an array of ipconfigs to the CNS Service.
//Used to add IPConfigs to the CNS pool, specifically in the scenario of rebatching.
func (service *HTTPRestService) AddIPConfigsToState(ipconfigs []*cns.ContainerIPConfigState) error {
func (service *HTTPRestService) addIPConfigsToState(ipconfigs map[string]cns.ContainerIPConfigState) error {
var (
err error
index int
ipconfig *cns.ContainerIPConfigState
ipconfig cns.ContainerIPConfigState
)

addedIPconfigs := make([]cns.ContainerIPConfigState, 0)

service.Lock()

defer func() {
service.Unlock()

if err != nil {
if removeErr := service.RemoveIPConfigsFromState(ipconfigs[0:index]); removeErr != nil {
if removeErr := service.removeIPConfigsFromState(addedIPconfigs); removeErr != nil {
logger.Printf("Failed remove IPConfig after AddIpConfigs: %v", removeErr)
}
}
}()

for index, ipconfig = range ipconfigs {
// ensure the ipconfigs we are not attempting to overwrite existing ipconfig state
existingIPConfigs := filterIPConfigMap(ipconfigs, func(ipconfig *cns.ContainerIPConfigState) bool {
existingIPConfig, exists := service.PodIPConfigState[ipconfig.ID]
if exists && existingIPConfig.State != ipconfig.State {
return true
}
return false
})
if len(existingIPConfigs) > 0 {
return fmt.Errorf("Failed to add IPConfigs to state, attempting to overwrite existing ipconfig states: %v", existingIPConfigs)
}

// add ipconfigs to state
for _, ipconfig = range ipconfigs {
if err = validateIPConfig(ipconfig); err != nil {
return err
}

service.PodIPConfigState[ipconfig.ID] = ipconfig
addedIPconfigs = append(addedIPconfigs, ipconfig)

if ipconfig.State == cns.Allocated {
var podInfo cns.KubernetesPodInfo
Expand All @@ -179,9 +198,35 @@ func (service *HTTPRestService) AddIPConfigsToState(ipconfigs []*cns.ContainerIP
return err
}

func filterIPConfigMap(toBeAdded map[string]cns.ContainerIPConfigState, f func(*cns.ContainerIPConfigState) bool) []*cns.ContainerIPConfigState {
vsf := make([]*cns.ContainerIPConfigState, 0)
for _, v := range toBeAdded {
if f(&v) {
vsf = append(vsf, &v)
}
}
return vsf
}

func (service *HTTPRestService) GetAllocatedIPConfigs() []*cns.ContainerIPConfigState {
service.RLock()
defer service.RUnlock()
return filterIPConfigMap(service.PodIPConfigState, func(ipconfig *cns.ContainerIPConfigState) bool {
return ipconfig.State == cns.Allocated
})
}

func (service *HTTPRestService) GetAvailableIPConfigs() []*cns.ContainerIPConfigState {
service.RLock()
defer service.RUnlock()
return filterIPConfigMap(service.PodIPConfigState, func(ipconfig *cns.ContainerIPConfigState) bool {
return ipconfig.State == cns.Available
})
}

//RemoveIPConfigsFromState takes a lock on the service object, and will remove an array of ipconfigs to the CNS Service.
//Used to add IPConfigs to the CNS pool, specifically in the scenario of rebatching.
func (service *HTTPRestService) RemoveIPConfigsFromState(ipconfigs []*cns.ContainerIPConfigState) error {
func (service *HTTPRestService) removeIPConfigsFromState(ipconfigs []cns.ContainerIPConfigState) error {
service.Lock()
defer service.Unlock()

Expand All @@ -201,7 +246,7 @@ func (service *HTTPRestService) RemoveIPConfigsFromState(ipconfigs []*cns.Contai
}

//SetIPConfigAsAllocated takes a lock of the service, and sets the ipconfig in the CNS state as allocated, does not take a lock
func (service *HTTPRestService) setIPConfigAsAllocated(ipconfig *cns.ContainerIPConfigState, podInfo cns.KubernetesPodInfo, marshalledOrchestratorContext json.RawMessage) *cns.ContainerIPConfigState {
func (service *HTTPRestService) setIPConfigAsAllocated(ipconfig cns.ContainerIPConfigState, podInfo cns.KubernetesPodInfo, marshalledOrchestratorContext json.RawMessage) cns.ContainerIPConfigState {
ipconfig.State = cns.Allocated
ipconfig.OrchestratorContext = marshalledOrchestratorContext
service.PodIPIDByOrchestratorContext[podInfo.GetOrchestratorContextKey()] = ipconfig.ID
Expand All @@ -210,7 +255,7 @@ func (service *HTTPRestService) setIPConfigAsAllocated(ipconfig *cns.ContainerIP
}

//SetIPConfigAsAllocated and sets the ipconfig in the CNS state as allocated, does not take a lock
func (service *HTTPRestService) setIPConfigAsAvailable(ipconfig *cns.ContainerIPConfigState, podInfo cns.KubernetesPodInfo) *cns.ContainerIPConfigState {
func (service *HTTPRestService) setIPConfigAsAvailable(ipconfig cns.ContainerIPConfigState, podInfo cns.KubernetesPodInfo) cns.ContainerIPConfigState {
ipconfig.State = cns.Available
ipconfig.OrchestratorContext = nil
service.PodIPConfigState[ipconfig.ID] = ipconfig
Expand Down Expand Up @@ -238,9 +283,9 @@ func (service *HTTPRestService) ReleaseIPConfig(podInfo cns.KubernetesPodInfo) e
return nil
}

func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.KubernetesPodInfo) (*cns.ContainerIPConfigState, bool, error) {
func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.KubernetesPodInfo) (cns.ContainerIPConfigState, bool, error) {
var (
ipState *cns.ContainerIPConfigState
ipState cns.ContainerIPConfigState
isExist bool
)

Expand All @@ -259,8 +304,8 @@ func (service *HTTPRestService) GetExistingIPConfig(podInfo cns.KubernetesPodInf
return ipState, isExist, nil
}

func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.KubernetesPodInfo, desiredIPAddress string, orchestratorContext json.RawMessage) (*cns.ContainerIPConfigState, error) {
var ipState *cns.ContainerIPConfigState
func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.KubernetesPodInfo, desiredIPAddress string, orchestratorContext json.RawMessage) (cns.ContainerIPConfigState, error) {
var ipState cns.ContainerIPConfigState

service.Lock()
defer service.Unlock()
Expand All @@ -276,8 +321,8 @@ func (service *HTTPRestService) AllocateDesiredIPConfig(podInfo cns.KubernetesPo
return ipState, fmt.Errorf("Requested IP not found in pool")
}

func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.KubernetesPodInfo, orchestratorContext json.RawMessage) (*cns.ContainerIPConfigState, error) {
var ipState *cns.ContainerIPConfigState
func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.KubernetesPodInfo, orchestratorContext json.RawMessage) (cns.ContainerIPConfigState, error) {
var ipState cns.ContainerIPConfigState

service.Lock()
defer service.Unlock()
Expand All @@ -291,10 +336,10 @@ func (service *HTTPRestService) AllocateAnyAvailableIPConfig(podInfo cns.Kuberne
}

// If IPConfig is already allocated for pod, it returns that else it returns one of the available ipconfigs.
func requestIPConfigHelper(service *HTTPRestService, req cns.GetIPConfigRequest) (*cns.ContainerIPConfigState, error) {
func requestIPConfigHelper(service *HTTPRestService, req cns.GetIPConfigRequest) (cns.ContainerIPConfigState, error) {
var (
podInfo cns.KubernetesPodInfo
ipState *cns.ContainerIPConfigState
ipState cns.ContainerIPConfigState
isExist bool
err error
)
Expand Down
Loading