* add note in comment

* fix ipset metrics, add TODO comments, and add delete cache

* linux ipsetmanager

* Revert "add note in comment"

This reverts commit b20c486cfa.

* update comments and add stub for destroyNPMIPsets() func

* move restore file logic to external package

* remove existence check in updateDirtyCache()

* rearranging stuff and updates for old version of new ipsetmanager

* update ipsetmanager to version in master (from ipsetmanager-update PR)

* updates to ipsetmanager linux test

* revamped file creator for retry logic (for ipset and iptables restore)

* renaming variables and moving code around

* completed logic for retrying (generic and for ipset restore)

* addressing sectionID comment and including a call to errorHandler Callback

* remove obsolete struct fields

* Revert "remove obsolete struct fields"

This reverts commit b53af2c2d7.

* unit tests and some changes/reordering of code for file-creator

* file creator unit tests (forgot to add in last commit)

* refactored file creator for external testing of error handling

* added missing argument to fucntion

* first pass at unit testing for ipsetmanager_linux

* fix file creator a bit (use ioshim, etc.) and finish basic UTs for ipsetmanager linux

* implemented ApplyAll mode

* update error messages

* full UT coverage for file creator

* update comments, var names, and remove debug code

* full UT coverage for ipsetmanager linux

* resolvee golint problems with function literals

* resolve golint problems with function literals v2

* rename variable

* added reboot function to generic ipsetmanager

* added caveat about ApplyAll mode

* initialize metrics in dataplane constructor

* fix go lints and update error messages in file creator

* add basic integration testing file

* addressing comments

* fix unit tests

* fix lints in integration test file

* fixed bad function name in windows ipsetmanager
This commit is contained in:
Hunter Gregory 2021-10-15 16:55:35 -07:00 коммит произвёл GitHub
Родитель 279911c94a
Коммит e692542c0c
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
11 изменённых файлов: 1353 добавлений и 38 удалений

Просмотреть файл

@ -5,6 +5,7 @@ import (
"net"
"github.com/Azure/azure-container-networking/common"
"github.com/Azure/azure-container-networking/npm/metrics"
"github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets"
"github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies"
"github.com/Azure/azure-container-networking/npm/util"
@ -17,6 +18,11 @@ const (
AzureNetworkName = "azure"
)
var iMgrDefaultCfg = &ipsets.IPSetManagerCfg{
IPSetMode: ipsets.ApplyOnNeed,
NetworkName: AzureNetworkName,
}
type DataPlane struct {
policyMgr *policies.PolicyManager
ipsetMgr *ipsets.IPSetManager
@ -49,9 +55,10 @@ type UpdateNPMPod struct {
}
func NewDataPlane(nodeName string, ioShim *common.IOShim) *DataPlane {
metrics.InitializeAll()
return &DataPlane{
policyMgr: policies.NewPolicyManager(ioShim),
ipsetMgr: ipsets.NewIPSetManager(AzureNetworkName, ioShim),
ipsetMgr: ipsets.NewIPSetManager(iMgrDefaultCfg, ioShim),
endpointCache: make(map[string]*NPMEndpoint),
nodeName: nodeName,
ioShim: ioShim,

Просмотреть файл

@ -7,13 +7,20 @@ import (
"github.com/Azure/azure-container-networking/npm/metrics"
"github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets"
"github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies"
"github.com/Azure/azure-container-networking/npm/util"
testutils "github.com/Azure/azure-container-networking/test/utils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
var (
ioShim = common.NewMockIOShim([]testutils.TestCmd{})
fakeIPSetRestoreSuccess = testutils.TestCmd{
Cmd: []string{util.Ipset, util.IpsetRestoreFlag},
ExitCode: 0,
}
emptyMockIOShim = common.NewMockIOShim([]testutils.TestCmd{})
setPodKey1 = &ipsets.TranslatedIPSet{
Metadata: ipsets.NewIPSetMetadata("setpodkey1", ipsets.KeyLabelOfPod),
}
@ -60,7 +67,7 @@ var (
func TestNewDataPlane(t *testing.T) {
metrics.InitializeAll()
dp := NewDataPlane("testnode", ioShim)
dp := NewDataPlane("testnode", emptyMockIOShim)
if dp == nil {
t.Error("NewDataPlane() returned nil")
@ -72,7 +79,7 @@ func TestNewDataPlane(t *testing.T) {
func TestInitializeDataPlane(t *testing.T) {
metrics.InitializeAll()
dp := NewDataPlane("testnode", ioShim)
dp := NewDataPlane("testnode", emptyMockIOShim)
assert.NotNil(t, dp)
err := dp.InitializeDataPlane()
@ -81,7 +88,7 @@ func TestInitializeDataPlane(t *testing.T) {
func TestResetDataPlane(t *testing.T) {
metrics.InitializeAll()
dp := NewDataPlane("testnode", ioShim)
dp := NewDataPlane("testnode", emptyMockIOShim)
assert.NotNil(t, dp)
err := dp.InitializeDataPlane()
@ -92,7 +99,7 @@ func TestResetDataPlane(t *testing.T) {
func TestCreateAndDeleteIpSets(t *testing.T) {
metrics.InitializeAll()
dp := NewDataPlane("testnode", ioShim)
dp := NewDataPlane("testnode", emptyMockIOShim)
assert.NotNil(t, dp)
setsTocreate := []*ipsets.IPSetMetadata{
{
@ -133,7 +140,7 @@ func TestCreateAndDeleteIpSets(t *testing.T) {
func TestAddToSet(t *testing.T) {
metrics.InitializeAll()
dp := NewDataPlane("testnode", ioShim)
dp := NewDataPlane("testnode", emptyMockIOShim)
setsTocreate := []*ipsets.IPSetMetadata{
{
@ -192,6 +199,8 @@ func TestAddToSet(t *testing.T) {
func TestApplyPolicy(t *testing.T) {
metrics.InitializeAll()
calls := []testutils.TestCmd{fakeIPSetRestoreSuccess}
ioShim := common.NewMockIOShim(calls)
dp := NewDataPlane("testnode", ioShim)
err := dp.AddPolicy(testPolicyobj)
@ -200,6 +209,8 @@ func TestApplyPolicy(t *testing.T) {
func TestRemovePolicy(t *testing.T) {
metrics.InitializeAll()
calls := []testutils.TestCmd{fakeIPSetRestoreSuccess, fakeIPSetRestoreSuccess}
ioShim := common.NewMockIOShim(calls)
dp := NewDataPlane("testnode", ioShim)
err := dp.AddPolicy(testPolicyobj)
@ -211,6 +222,8 @@ func TestRemovePolicy(t *testing.T) {
func TestUpdatePolicy(t *testing.T) {
metrics.InitializeAll()
calls := []testutils.TestCmd{fakeIPSetRestoreSuccess, fakeIPSetRestoreSuccess}
ioShim := common.NewMockIOShim(calls)
dp := NewDataPlane("testnode", ioShim)
err := dp.AddPolicy(testPolicyobj)

Просмотреть файл

@ -0,0 +1,270 @@
package ioutil
import (
"bytes"
"fmt"
"regexp"
"strconv"
"strings"
"github.com/Azure/azure-container-networking/common"
"github.com/Azure/azure-container-networking/log"
npmerrors "github.com/Azure/azure-container-networking/npm/util/errors"
)
// TODO add file creator log prefix
// FileCreator is a tool for:
// - building a buffer file
// - running a command with the file
// - handling errors in the file
type FileCreator struct {
lines []*Line
sections map[string]*Section // key is sectionID
lineNumbersToOmit map[int]struct{}
errorsToRetryOn []*ErrorDefinition
lineFailureDefinitions []*ErrorDefinition
tryCount int
maxTryCount int
ioShim *common.IOShim
}
// TODO for iptables:
// lineFailurePattern := "line (\\d+) failed"
// AND "Error occurred at line: (\\d+)"
// Line defines the content, section, and error handlers for a line
type Line struct {
content string
sectionID string
errorHandlers []*LineErrorHandler
}
// Section is a logically connected components (not necessarily adjacent lines)
type Section struct {
id string
lineNums []int
}
// ErrorDefinition defines an error by a regular expression and its error code.
type ErrorDefinition struct {
matchPattern string
re *regexp.Regexp
}
// LineErrorHandler defines an error and how to handle it
type LineErrorHandler struct {
Definition *ErrorDefinition
Method LineErrorHandlerMethod
Reason string
Callback func()
}
// LineErrorHandlerMethod defines behavior when an error occurs
type LineErrorHandlerMethod string
// possible LineErrorHandlerMethod
const (
SkipLine LineErrorHandlerMethod = "skip"
AbortSection LineErrorHandlerMethod = "abort"
)
func NewFileCreator(ioShim *common.IOShim, maxTryCount int, lineFailurePatterns ...string) *FileCreator {
creator := &FileCreator{
lines: make([]*Line, 0),
sections: make(map[string]*Section),
lineNumbersToOmit: make(map[int]struct{}),
errorsToRetryOn: make([]*ErrorDefinition, 0),
lineFailureDefinitions: make([]*ErrorDefinition, len(lineFailurePatterns)),
tryCount: 0,
maxTryCount: maxTryCount,
ioShim: ioShim,
}
for k, lineFailurePattern := range lineFailurePatterns {
creator.lineFailureDefinitions[k] = NewErrorDefinition(lineFailurePattern)
}
return creator
}
func NewErrorDefinition(pattern string) *ErrorDefinition {
return &ErrorDefinition{
matchPattern: pattern,
re: regexp.MustCompile(pattern),
}
}
func (creator *FileCreator) AddErrorToRetryOn(definition *ErrorDefinition) {
creator.errorsToRetryOn = append(creator.errorsToRetryOn, definition)
}
func (creator *FileCreator) AddLine(sectionID string, errorHandlers []*LineErrorHandler, items ...string) {
section, exists := creator.sections[sectionID]
if !exists {
section = &Section{sectionID, make([]int, 0)}
creator.sections[sectionID] = section
}
spaceSeparatedItems := strings.Join(items, " ")
line := &Line{spaceSeparatedItems, sectionID, errorHandlers}
creator.lines = append(creator.lines, line)
section.lineNums = append(section.lineNums, len(creator.lines)-1)
}
// ToString combines the lines in the FileCreator and ends with a new line.
func (creator *FileCreator) ToString() string {
result := strings.Builder{}
for lineNum, line := range creator.lines {
_, isOmitted := creator.lineNumbersToOmit[lineNum]
if !isOmitted {
result.WriteString(line.content + "\n")
}
}
return result.String()
}
func (creator *FileCreator) RunCommandWithFile(cmd string, args ...string) error {
fileString := creator.ToString()
wasFileAltered, err := creator.runCommandOnceWithFile(fileString, cmd, args...)
if err == nil {
// success
return nil
}
for {
commandString := cmd + " " + strings.Join(args, " ")
if creator.hasNoMoreRetries() {
// TODO conditionally specify as retriable?
return npmerrors.Errorf(npmerrors.RunFileCreator, false, fmt.Sprintf("failed to run command [%s] with error: %v", commandString, err))
}
if wasFileAltered {
fileString = creator.ToString()
log.Logf("rerunning command [%s] with new file:\n%s", commandString, fileString)
} else {
log.Logf("rerunning command [%s] with the same file", commandString)
}
wasFileAltered, err = creator.runCommandOnceWithFile(fileString, cmd, args...)
if err == nil {
// success
return nil
}
}
}
// RunCommandOnceWithFile runs the command with the file once and increments the try count.
// It returns whether the file was altered and any error.
// For automatic retrying and proper logging, use RunCommandWithFile.
// This method can be used for external testing of file creator contents after each run.
func (creator *FileCreator) RunCommandOnceWithFile(cmd string, args ...string) (bool, error) {
if creator.hasNoMoreRetries() {
return false, npmerrors.Errorf(npmerrors.RunFileCreator, false, fmt.Sprintf("reached max try count %d", creator.tryCount))
}
fileString := creator.ToString()
return creator.runCommandOnceWithFile(fileString, cmd, args...)
}
// TODO return another bool that specifies if there was a file-level retriable error?
func (creator *FileCreator) runCommandOnceWithFile(fileString, cmd string, args ...string) (bool, error) {
command := creator.ioShim.Exec.Command(cmd, args...)
command.SetStdin(bytes.NewBufferString(fileString))
// run the command
stdErrBytes, err := command.CombinedOutput()
if err == nil {
// success
return false, nil
}
creator.tryCount++
commandString := cmd + " " + strings.Join(args, " ")
stdErr := string(stdErrBytes)
log.Errorf("on try number %d, failed to run command [%s] with error [%v] and stdErr [%s]. Used file:\n%s", creator.tryCount, commandString, err, stdErr, fileString)
if creator.hasNoMoreRetries() {
return false, fmt.Errorf("after %d tries, failed with final error [%w] and stdErr [%s]", creator.tryCount, err, stdErr)
}
// begin the retry logic
if creator.hasFileLevelError(stdErr) {
log.Logf("detected a file-level error after running command [%s]", commandString)
return false, fmt.Errorf("file-level error: %w", err)
}
// no file-level error, so handle line-level error if there is one
lineNum := creator.getErrorLineNumber(commandString, stdErr)
if lineNum == -1 {
// can't detect a line number error
return false, fmt.Errorf("can't discern error: %w", err)
}
wasFileAltered := creator.handleLineError(lineNum, commandString, stdErr)
return wasFileAltered, fmt.Errorf("tried to handle line number error: %w", err)
}
func (creator *FileCreator) hasNoMoreRetries() bool {
return creator.tryCount >= creator.maxTryCount
}
func (creator *FileCreator) hasFileLevelError(stdErr string) bool {
for _, errorDefinition := range creator.errorsToRetryOn {
if errorDefinition.isMatch(stdErr) {
return true
}
}
return false
}
func (definition *ErrorDefinition) isMatch(stdErr string) bool {
return definition.re.MatchString(stdErr)
}
// return -1 if there's a failure
func (creator *FileCreator) getErrorLineNumber(commandString, stdErr string) int {
for _, definition := range creator.lineFailureDefinitions {
result := definition.re.FindStringSubmatch(stdErr)
if result == nil || len(result) < 2 {
log.Logf("expected error with line number, but couldn't detect one with error regex pattern [%s] for command [%s] with stdErr [%s]", definition.matchPattern, commandString, stdErr)
continue
}
lineNumString := result[1]
lineNum, err := strconv.Atoi(lineNumString)
if err != nil {
log.Logf("expected error with line number, but error regex pattern %s didn't produce a number for command [%s] with stdErr [%s]", definition.matchPattern, commandString, stdErr)
continue
}
if lineNum < 1 || lineNum > len(creator.lines) {
log.Logf("expected error with line number, but error regex pattern %s produced an invalid line number %d for command [%s] with stdErr [%s]", definition.matchPattern, lineNum, commandString, stdErr)
continue
}
return lineNum
}
return -1
}
// return whether the file was altered
func (creator *FileCreator) handleLineError(lineNum int, commandString, stdErr string) bool {
lineNumIndex := lineNum - 1
line := creator.lines[lineNumIndex]
for _, errorHandler := range line.errorHandlers {
if !errorHandler.Definition.isMatch(stdErr) {
continue
}
switch errorHandler.Method {
case SkipLine:
log.Errorf("skipping line %d for command [%s]", lineNumIndex, commandString)
creator.lineNumbersToOmit[lineNumIndex] = struct{}{}
errorHandler.Callback()
return true
case AbortSection:
log.Errorf("aborting section associated with line %d for command [%s]", lineNumIndex, commandString)
section, exists := creator.sections[line.sectionID]
if !exists {
log.Errorf("can't abort section because line references section %d which doesn't exist, so skipping the line instead", line.sectionID)
creator.lineNumbersToOmit[lineNumIndex] = struct{}{}
} else {
for _, lineNum := range section.lineNums {
creator.lineNumbersToOmit[lineNum] = struct{}{}
}
}
}
errorHandler.Callback()
return true
}
log.Logf("no error handler for line %d for command [%s] with stdErr [%s]", lineNum, commandString, stdErr)
return false
}

Просмотреть файл

@ -0,0 +1,260 @@
package ioutil
import (
"fmt"
"testing"
"github.com/Azure/azure-container-networking/common"
"github.com/Azure/azure-container-networking/log"
testutils "github.com/Azure/azure-container-networking/test/utils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const (
testCommandString = "test command"
section1ID = "section1"
section2ID = "section2"
)
var fakeSuccessCommand = testutils.TestCmd{
Cmd: []string{testCommandString},
Stdout: "success",
ExitCode: 0,
}
func TestToStringAndSections(t *testing.T) {
creator := NewFileCreator(common.NewMockIOShim(nil), 1)
creator.AddLine(section1ID, nil, "line1-item1", "line1-item2", "line1-item3")
creator.AddLine(section2ID, nil, "line2-item1", "line2-item2", "line2-item3")
creator.AddLine(section1ID, nil, "line3-item1", "line3-item2", "line3-item3")
section1 := creator.sections[section1ID]
require.Equal(t, section1ID, section1.id)
require.Equal(t, []int{0, 2}, section1.lineNums)
section2 := creator.sections[section2ID]
require.Equal(t, section2ID, section2.id)
require.Equal(t, []int{1}, section2.lineNums)
fileString := creator.ToString()
assert.Equal(
t,
`line1-item1 line1-item2 line1-item3
line2-item1 line2-item2 line2-item3
line3-item1 line3-item2 line3-item3
`,
fileString,
)
}
func TestRunCommandWithFile(t *testing.T) {
calls := []testutils.TestCmd{fakeSuccessCommand}
creator := NewFileCreator(common.NewMockIOShim(calls), 1)
require.NoError(t, creator.RunCommandWithFile(testCommandString))
}
func TestRecoveryForFileLevelError(t *testing.T) {
calls := []testutils.TestCmd{
{
Cmd: []string{testCommandString},
Stdout: "file-level error",
ExitCode: 4,
},
fakeSuccessCommand,
}
creator := NewFileCreator(common.NewMockIOShim(calls), 2)
creator.AddErrorToRetryOn(NewErrorDefinition("file-level error"))
require.NoError(t, creator.RunCommandWithFile(testCommandString))
}
func TestRecoveryForLineError(t *testing.T) {
calls := []testutils.TestCmd{
{
Cmd: []string{testCommandString},
Stdout: "failure on line 2",
ExitCode: 4,
},
fakeSuccessCommand,
}
creator := NewFileCreator(common.NewMockIOShim(calls), 2, "failure on line (\\d+)")
require.NoError(t, creator.RunCommandWithFile(testCommandString))
}
func TestTotalFailureAfterRetries(t *testing.T) {
errorCommand := testutils.TestCmd{
Cmd: []string{testCommandString},
Stdout: "some error",
ExitCode: 4,
}
calls := []testutils.TestCmd{errorCommand, errorCommand, errorCommand}
creator := NewFileCreator(common.NewMockIOShim(calls), 2)
require.Error(t, creator.RunCommandWithFile(testCommandString))
}
func TestHandleLineErrorForAbortSection(t *testing.T) {
fakeErrorCommand := testutils.TestCmd{
Cmd: []string{testCommandString},
Stdout: "failure on line 1: match-pattern do something please",
ExitCode: 1,
}
calls := []testutils.TestCmd{fakeErrorCommand}
creator := NewFileCreator(common.NewMockIOShim(calls), 2, "failure on line (\\d+)")
errorHandlers := []*LineErrorHandler{
// first error handler doesn't match (include this to make sure the real match gets reached)
{
Definition: NewErrorDefinition("abc"),
Method: AbortSection,
Reason: "",
Callback: func() {},
},
{
Definition: NewErrorDefinition("match-pattern"),
Method: AbortSection,
Reason: "error requiring us to abort section",
Callback: func() { log.Logf("abort section callback") },
},
}
creator.AddLine(section1ID, errorHandlers, "line1-item1", "line1-item2", "line1-item3")
creator.AddLine(section2ID, nil, "line2-item1", "line2-item2", "line2-item3")
creator.AddLine(section1ID, nil, "line3-item1", "line3-item2", "line3-item3")
wasFileAltered, err := creator.RunCommandOnceWithFile(testCommandString)
require.Error(t, err)
require.True(t, wasFileAltered)
fileString := creator.ToString()
assert.Equal(t, "line2-item1 line2-item2 line2-item3\n", fileString)
}
func TestHandleLineErrorForSkipLine(t *testing.T) {
fakeErrorCommand := testutils.TestCmd{
Cmd: []string{testCommandString},
Stdout: "failure on line 2: match-pattern do something please",
ExitCode: 1,
}
calls := []testutils.TestCmd{fakeErrorCommand}
creator := NewFileCreator(common.NewMockIOShim(calls), 2, "failure on line (\\d+)")
errorHandlers := []*LineErrorHandler{
{
Definition: NewErrorDefinition("match-pattern"),
Method: SkipLine,
Reason: "error requiring us to skip this line",
Callback: func() { log.Logf("skip line callback") },
},
}
creator.AddLine("", nil, "line1-item1", "line1-item2", "line1-item3")
creator.AddLine("", errorHandlers, "line2-item1", "line2-item2", "line2-item3")
creator.AddLine("", nil, "line3-item1", "line3-item2", "line3-item3")
wasFileAltered, err := creator.RunCommandOnceWithFile(testCommandString)
require.Error(t, err)
require.True(t, wasFileAltered)
fileString := creator.ToString()
assert.Equal(t, "line1-item1 line1-item2 line1-item3\nline3-item1 line3-item2 line3-item3\n", fileString)
}
func TestHandleLineErrorNoMatch(t *testing.T) {
fakeErrorCommand := testutils.TestCmd{
Cmd: []string{testCommandString},
Stdout: "failure on line 2: match-pattern do something please",
ExitCode: 1,
}
calls := []testutils.TestCmd{fakeErrorCommand}
creator := NewFileCreator(common.NewMockIOShim(calls), 2, "failure on line (\\d+)")
errorHandlers := []*LineErrorHandler{
{
Definition: NewErrorDefinition("abc"),
Method: AbortSection,
Reason: "",
Callback: func() {},
},
}
creator.AddLine("", nil, "line1-item1", "line1-item2", "line1-item3")
creator.AddLine("", errorHandlers, "line2-item1", "line2-item2", "line2-item3")
creator.AddLine("", nil, "line3-item1", "line3-item2", "line3-item3")
fileStringBefore := creator.ToString()
wasFileAltered, err := creator.RunCommandOnceWithFile(testCommandString)
require.Error(t, err)
require.False(t, wasFileAltered)
fileStringAfter := creator.ToString()
require.Equal(t, fileStringBefore, fileStringAfter)
}
func TestGetErrorLineNumber(t *testing.T) {
type args struct {
lineFailurePatterns []string
stdErr string
}
tests := []struct {
name string
args args
expectedLineNum int
}{
{
"pattern that doesn't match",
args{
[]string{"abc"},
"xyz",
},
-1,
},
{
"matching pattern with no group",
args{
[]string{"abc"},
"abc",
},
-1,
},
{
"matching pattern with non-numeric group",
args{
[]string{"(abc)"},
"abc",
},
-1,
},
{
"stderr gives an out-of-bounds line number",
args{
[]string{"line (\\d+)"},
"line 777",
},
-1,
},
{
"good line match",
args{
[]string{"line (\\d+)"},
`there was a failure
on line 11 where the failure happened
fix it please`,
},
11,
},
{
"good line match with other pattern that doesn't match",
args{
[]string{"abc", "line (\\d+)"},
`there was a failure
on line 11 where the failure happened
fix it please`,
},
11,
},
}
commandString := "test command"
for _, tt := range tests {
lineFailurePatterns := tt.args.lineFailurePatterns
expectedLineNum := tt.expectedLineNum
stdErr := tt.args.stdErr
t.Run(tt.name, func(t *testing.T) {
creator := NewFileCreator(common.NewMockIOShim(nil), 2, lineFailurePatterns...)
for i := 0; i < 15; i++ {
creator.AddLine("", nil, fmt.Sprintf("line%d", i))
}
lineNum := creator.getErrorLineNumber(commandString, stdErr)
require.Equal(t, expectedLineNum, lineNum)
})
}
}

Просмотреть файл

@ -21,7 +21,7 @@ const (
)
type IPSetManager struct {
iMgrCfg *ipSetManagerCfg
iMgrCfg *IPSetManagerCfg
setMap map[string]*IPSet
// Map with Key as IPSet name to to emulate set
// and value as struct{} for minimal memory consumption.
@ -32,17 +32,14 @@ type IPSetManager struct {
sync.Mutex
}
type ipSetManagerCfg struct {
ipSetMode IPSetMode
networkName string
type IPSetManagerCfg struct {
IPSetMode IPSetMode
NetworkName string
}
func NewIPSetManager(networkName string, ioShim *common.IOShim) *IPSetManager {
func NewIPSetManager(iMgrCfg *IPSetManagerCfg, ioShim *common.IOShim) *IPSetManager {
return &IPSetManager{
iMgrCfg: &ipSetManagerCfg{
ipSetMode: ApplyOnNeed,
networkName: networkName,
},
iMgrCfg: iMgrCfg,
setMap: make(map[string]*IPSet),
toAddOrUpdateCache: make(map[string]struct{}),
toDeleteCache: make(map[string]struct{}),
@ -50,6 +47,16 @@ func NewIPSetManager(networkName string, ioShim *common.IOShim) *IPSetManager {
}
}
func (iMgr *IPSetManager) ResetIPSets() error {
iMgr.Lock()
defer iMgr.Unlock()
err := iMgr.resetIPSets()
if err != nil {
return fmt.Errorf("error while resetting ipsetmanager: %w", err)
}
return nil
}
func (iMgr *IPSetManager) CreateIPSet(setMetadata *IPSetMetadata) {
iMgr.Lock()
defer iMgr.Unlock()
@ -59,6 +66,9 @@ func (iMgr *IPSetManager) CreateIPSet(setMetadata *IPSetMetadata) {
}
iMgr.setMap[prefixedName] = NewIPSet(setMetadata)
metrics.IncNumIPSets()
if iMgr.iMgrCfg.IPSetMode == ApplyAllIPSets {
iMgr.modifyCacheForKernelCreation(prefixedName)
}
}
// DeleteIPSet expects the prefixed ipset name
@ -74,9 +84,12 @@ func (iMgr *IPSetManager) DeleteIPSet(name string) {
return
}
// the set will not be in the kernel since there are no references, so there's no need to update the dirty cache
delete(iMgr.setMap, name)
metrics.DecNumIPSets()
if iMgr.iMgrCfg.IPSetMode == ApplyAllIPSets {
iMgr.modifyCacheForKernelRemoval(name) // FIXME this mode would try to delete an ipset from the kernel if it's never been created in the kernel
}
// if mode is ApplyOnNeed, the set will not be in the kernel (or will be in the delete cache already) since there are no references
}
// GetIPSet needs the prefixed ipset name
@ -105,9 +118,10 @@ func (iMgr *IPSetManager) AddReference(setName, referenceName string, referenceT
if referenceType == SelectorType && !set.canSetBeSelectorIPSet() {
return npmerrors.Errorf(npmerrors.AddSelectorReference, false, fmt.Sprintf("ipset %s is not a selector ipset it is of type %s", setName, set.Type.String()))
}
wasInKernel := set.shouldBeInKernel()
wasInKernel := iMgr.shouldBeInKernel(set)
set.addReference(referenceName, referenceType)
if !wasInKernel {
// the set should be in the kernel, so add it to the kernel if it wasn't beforehand
iMgr.modifyCacheForKernelCreation(set.Name)
// if set.Kind == HashSet, then this for loop will do nothing
@ -131,9 +145,10 @@ func (iMgr *IPSetManager) DeleteReference(setName, referenceName string, referen
}
set := iMgr.setMap[setName]
wasInKernel := set.shouldBeInKernel() // required because the set may have 0 references i.e. this reference doesn't exist
wasInKernel := iMgr.shouldBeInKernel(set) // required because the set may not be in the kernel if this reference doesn't exist
set.deleteReference(referenceName, referenceType)
if wasInKernel && !set.shouldBeInKernel() {
if wasInKernel && !iMgr.shouldBeInKernel(set) {
// remove from kernel if it was in the kernel before and shouldn't be now
iMgr.modifyCacheForKernelRemoval(set.Name)
// if set.Kind == HashSet, then this for loop will do nothing
@ -241,6 +256,8 @@ func (iMgr *IPSetManager) ApplyIPSets(networkID string) error {
iMgr.Lock()
defer iMgr.Unlock()
fmt.Println(networkID) // FIXME remove
// Call the appropriate apply ipsets
err := iMgr.applyIPSets()
if err != nil {
@ -317,13 +334,17 @@ func (iMgr *IPSetManager) modifyCacheForKernelCreation(setName string) {
}
func (iMgr *IPSetManager) incKernelReferCountAndModifyCache(member *IPSet) {
wasInKernel := member.shouldBeInKernel()
wasInKernel := iMgr.shouldBeInKernel(member)
member.incKernelReferCount()
if !wasInKernel {
iMgr.modifyCacheForKernelCreation(member.Name)
}
}
func (iMgr *IPSetManager) shouldBeInKernel(set *IPSet) bool {
return set.shouldBeInKernel() || iMgr.iMgrCfg.IPSetMode == ApplyAllIPSets
}
func (iMgr *IPSetManager) modifyCacheForKernelRemoval(setName string) {
iMgr.toDeleteCache[setName] = struct{}{}
delete(iMgr.toAddOrUpdateCache, setName)
@ -338,7 +359,7 @@ func (iMgr *IPSetManager) modifyCacheForKernelRemoval(setName string) {
func (iMgr *IPSetManager) decKernelReferCountAndModifyCache(member *IPSet) {
member.decKernelReferCount()
if !member.shouldBeInKernel() {
if !iMgr.shouldBeInKernel(member) {
iMgr.modifyCacheForKernelRemoval(member.Name)
}
}
@ -360,7 +381,7 @@ func (iMgr *IPSetManager) checkForIPUpdateErrors(setNames []*IPSetMetadata, npmE
func (iMgr *IPSetManager) modifyCacheForKernelMemberUpdate(setName string) {
set := iMgr.setMap[setName]
if set.shouldBeInKernel() {
if iMgr.shouldBeInKernel(set) {
iMgr.toAddOrUpdateCache[setName] = struct{}{}
/*
TODO kernel-based prometheus metrics
@ -415,7 +436,7 @@ func (iMgr *IPSetManager) addMemberIPSet(listName, memberName string) {
list.MemberIPSets[memberName] = member
member.incIPSetReferCount()
metrics.AddEntryToIPSet(list.Name)
listIsInKernel := list.shouldBeInKernel()
listIsInKernel := iMgr.shouldBeInKernel(list)
if listIsInKernel {
iMgr.incKernelReferCountAndModifyCache(member)
}
@ -431,7 +452,7 @@ func (iMgr *IPSetManager) removeMemberIPSet(listName, memberName string) {
delete(list.MemberIPSets, member.Name)
member.decIPSetReferCount()
metrics.RemoveEntryFromIPSet(list.Name)
listIsInKernel := list.shouldBeInKernel()
listIsInKernel := iMgr.shouldBeInKernel(list)
if listIsInKernel {
iMgr.decKernelReferCountAndModifyCache(member)
}

Просмотреть файл

@ -1,5 +1,181 @@
package ipsets
func (iMgr *IPSetManager) applyIPSets() error {
import (
"fmt"
"github.com/Azure/azure-container-networking/log"
"github.com/Azure/azure-container-networking/npm/pkg/dataplane/ioutil"
"github.com/Azure/azure-container-networking/npm/util"
)
const (
maxTryCount = 1
deletionPrefix = "delete"
creationPrefix = "create"
ipsetRestoreLineFailurePattern = "Error in line (\\d+):"
setAlreadyExistsPattern = "Set cannot be created: set with the same name already exists"
setDoesntExistPattern = "The set with the given name does not exist"
setInUseByKernelPattern = "Set cannot be destroyed: it is in use by a kernel component"
memberSetDoesntExist = "Set to be added/deleted/tested as element does not exist"
)
func (iMgr *IPSetManager) resetIPSets() error {
// called on failure or when NPM is created
// so no ipset cache. need to use ipset list like in ipsm.go
// create restore file that flushes all sets, then deletes all sets
// technically don't need to flush a hashset
return nil
}
// don't need networkID
func (iMgr *IPSetManager) applyIPSets() error {
toDeleteSetNames := convertAndDeleteCache(iMgr.toDeleteCache)
toAddOrUpdateSetNames := convertAndDeleteCache(iMgr.toAddOrUpdateCache)
creator := iMgr.getFileCreator(maxTryCount, toDeleteSetNames, toAddOrUpdateSetNames)
err := creator.RunCommandWithFile(util.Ipset, util.IpsetRestoreFlag)
if err != nil {
return fmt.Errorf("%w", err)
}
return nil
}
func convertAndDeleteCache(cache map[string]struct{}) []string {
result := make([]string, len(cache))
i := 0
for setName := range cache {
result[i] = setName
delete(cache, setName)
i++
}
return result
}
// getFileCreator encodes an ipset restore file with error handling.
// We use slices instead of maps so we can have determinstic behavior for
// unit tests on the file creator i.e. check file contents before and after error handling.
// Without slices, we could do unit tests on certain segments of the file,
// but things would get complicated for checking error handling.
// We can't escape the nondeterministic behavior of adding members,
// but we can handle this in UTs with sorting.
func (iMgr *IPSetManager) getFileCreator(maxTryCount int, toDeleteSetNames, toAddOrUpdateSetNames []string) *ioutil.FileCreator {
creator := ioutil.NewFileCreator(iMgr.ioShim, maxTryCount, ipsetRestoreLineFailurePattern)
// creator.AddErrorToRetryOn(ioutil.NewErrorDefinition("something")) // TODO add file-level errors?
iMgr.handleDeletions(creator, toDeleteSetNames)
iMgr.handleAddOrUpdates(creator, toAddOrUpdateSetNames)
return creator
}
func (iMgr *IPSetManager) handleDeletions(creator *ioutil.FileCreator, setNames []string) {
// flush all first so we don't try to delete an ipset referenced by a list we're deleting too
// error handling:
// - abort the flush and delete call for a set if the set doesn't exist
// - if the set is in use by a kernel component, then skip the delete and mark it as a failure
for _, setName := range setNames {
setName := setName // to appease golint complaints about function literal
errorHandlers := []*ioutil.LineErrorHandler{
{
Definition: ioutil.NewErrorDefinition(setDoesntExistPattern),
Method: ioutil.AbortSection,
Callback: func() {
// no action needed since we expect that it's gone after applyIPSets()
log.Logf("was going to delete set %s but it doesn't exist", setName)
},
},
}
sectionID := getSectionID(deletionPrefix, setName)
hashedSetName := util.GetHashedName(setName)
creator.AddLine(sectionID, errorHandlers, util.IpsetFlushFlag, hashedSetName) // flush set
}
for _, setName := range setNames {
setName := setName // to appease golint complaints about function literal
errorHandlers := []*ioutil.LineErrorHandler{
{
Definition: ioutil.NewErrorDefinition(setInUseByKernelPattern),
Method: ioutil.SkipLine,
Callback: func() {
log.Errorf("was going to delete set %s but it is in use by a kernel component", setName)
// TODO mark the set as a failure and reconcile what iptables rule or ipset is referring to it
},
},
}
sectionID := getSectionID(deletionPrefix, setName)
hashedSetName := util.GetHashedName(setName)
creator.AddLine(sectionID, errorHandlers, util.IpsetDestroyFlag, hashedSetName) // destroy set
}
}
func (iMgr *IPSetManager) handleAddOrUpdates(creator *ioutil.FileCreator, setNames []string) {
// create all sets first
// error handling:
// - abort the create, flush, and add calls if create doesn't work
// Won't abort adding the set to a list. Will need another retry to handle that
// TODO change this behavior?
for _, setName := range setNames {
set := iMgr.setMap[setName]
methodFlag := util.IpsetNetHashFlag
if set.Kind == ListSet {
methodFlag = util.IpsetSetListFlag
} else if set.Type == NamedPorts {
methodFlag = util.IpsetIPPortHashFlag
}
specs := []string{util.IpsetCreationFlag, set.HashedName, util.IpsetExistFlag, methodFlag}
if set.Type == CIDRBlocks {
specs = append(specs, util.IpsetMaxelemName, util.IpsetMaxelemNum)
}
setName := setName // to appease golint complaints about function literal
errorHandlers := []*ioutil.LineErrorHandler{
{
Definition: ioutil.NewErrorDefinition(setAlreadyExistsPattern),
Method: ioutil.AbortSection,
Callback: func() {
log.Errorf("was going to add/update set %s but couldn't create the set", setName)
// TODO mark the set as a failure and handle this
},
},
}
sectionID := getSectionID(creationPrefix, setName)
creator.AddLine(sectionID, errorHandlers, specs...) // create set
}
// flush and add all IPs/members for each set
// error handling:
// - if a member set can't be added to a list because it doesn't exist, then skip the add and mark it as a failure
for _, setName := range setNames {
set := iMgr.setMap[setName]
sectionID := getSectionID(creationPrefix, setName)
creator.AddLine(sectionID, nil, util.IpsetFlushFlag, set.HashedName) // flush set (no error handler needed)
if set.Kind == HashSet {
for ip := range set.IPPodKey {
// TODO add error handler?
creator.AddLine(sectionID, nil, util.IpsetAppendFlag, set.HashedName, ip) // add IP
}
} else {
setName := setName // to appease golint complaints about function literal
for _, member := range set.MemberIPSets {
memberName := member.Name // to appease golint complaints about function literal
errorHandlers := []*ioutil.LineErrorHandler{
{
Definition: ioutil.NewErrorDefinition(memberSetDoesntExist),
Method: ioutil.SkipLine,
Callback: func() {
log.Errorf("was going to add member set %s to list %s, but the member doesn't exist", memberName, setName)
// TODO handle error
},
},
}
creator.AddLine(sectionID, errorHandlers, util.IpsetAppendFlag, set.HashedName, member.HashedName) // add member
}
}
}
}
func getSectionID(prefix, setName string) string {
return fmt.Sprintf("%s-%s", prefix, setName)
}

Просмотреть файл

@ -0,0 +1,441 @@
package ipsets
import (
"fmt"
"sort"
"strings"
"testing"
"github.com/Azure/azure-container-networking/common"
"github.com/Azure/azure-container-networking/npm/pkg/dataplane/ioutil"
"github.com/Azure/azure-container-networking/npm/util"
testutils "github.com/Azure/azure-container-networking/test/utils"
"github.com/stretchr/testify/require"
)
type testSet struct {
metadata *IPSetMetadata
hashedName string
}
func createTestSet(name string, setType SetType) *testSet {
set := &testSet{
metadata: &IPSetMetadata{name, setType},
}
set.hashedName = util.GetHashedName(set.metadata.GetPrefixName())
return set
}
var (
iMgrApplyAllCfg = &IPSetManagerCfg{
IPSetMode: ApplyAllIPSets,
NetworkName: "",
}
ipsetRestoreStringSlice = []string{util.Ipset, util.IpsetRestoreFlag}
fakeRestoreSuccessCommand = testutils.TestCmd{
Cmd: ipsetRestoreStringSlice,
Stdout: "success",
ExitCode: 0,
}
testNSSet = createTestSet("test-ns-set", NameSpace)
testKeyPodSet = createTestSet("test-keyPod-set", KeyLabelOfPod)
testKVPodSet = createTestSet("test-kvPod-set", KeyValueLabelOfPod)
testNamedportSet = createTestSet("test-namedport-set", NamedPorts)
testCIDRSet = createTestSet("test-cidr-set", CIDRBlocks)
testKeyNSList = createTestSet("test-keyNS-list", KeyLabelOfNameSpace)
testKVNSList = createTestSet("test-kvNS-list", KeyValueLabelOfNameSpace)
testNestedLabelList = createTestSet("test-nestedlabel-list", NestedLabelOfPod)
)
func TestDestroyNPMIPSets(t *testing.T) {
calls := []testutils.TestCmd{} // TODO
iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls))
require.NoError(t, iMgr.resetIPSets())
}
func TestConvertAndDeleteCache(t *testing.T) {
cache := map[string]struct{}{
"a": {},
"b": {},
"c": {},
"d": {},
}
slice := convertAndDeleteCache(cache)
require.Equal(t, 0, len(cache))
require.Equal(t, 4, len(slice))
for _, item := range []string{"a", "b", "c", "d"} {
success := false
for _, sliceItem := range slice {
if item == sliceItem {
success = true
}
}
if !success {
require.FailNowf(t, "%s not in the slice", item)
}
}
}
// create all possible SetTypes
func TestApplyCreationsAndAdds(t *testing.T) {
calls := []testutils.TestCmd{fakeRestoreSuccessCommand}
iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls))
lines := []string{
fmt.Sprintf("-N %s -exist nethash", testNSSet.hashedName),
fmt.Sprintf("-N %s -exist nethash", testKeyPodSet.hashedName),
fmt.Sprintf("-N %s -exist nethash", testKVPodSet.hashedName),
fmt.Sprintf("-N %s -exist hash:ip,port", testNamedportSet.hashedName),
fmt.Sprintf("-N %s -exist nethash maxelem 4294967295", testCIDRSet.hashedName),
fmt.Sprintf("-N %s -exist setlist", testKeyNSList.hashedName),
fmt.Sprintf("-N %s -exist setlist", testKVNSList.hashedName),
fmt.Sprintf("-N %s -exist setlist", testNestedLabelList.hashedName),
}
lines = append(lines, getSortedLines(testNSSet, "10.0.0.0", "10.0.0.1")...)
lines = append(lines, getSortedLines(testKeyPodSet, "10.0.0.5")...)
lines = append(lines, getSortedLines(testKVPodSet)...)
lines = append(lines, getSortedLines(testNamedportSet)...)
lines = append(lines, getSortedLines(testCIDRSet)...)
lines = append(lines, getSortedLines(testKeyNSList, testNSSet.hashedName, testKeyPodSet.hashedName)...)
lines = append(lines, getSortedLines(testKVNSList, testKVPodSet.hashedName)...)
lines = append(lines, getSortedLines(testNestedLabelList)...)
expectedFileString := strings.Join(lines, "\n") + "\n"
iMgr.CreateIPSet(testNSSet.metadata)
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.0", "a"))
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.1", "b"))
iMgr.CreateIPSet(testKeyPodSet.metadata)
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testKeyPodSet.metadata}, "10.0.0.5", "c"))
iMgr.CreateIPSet(testKVPodSet.metadata)
iMgr.CreateIPSet(testNamedportSet.metadata)
iMgr.CreateIPSet(testCIDRSet.metadata)
iMgr.CreateIPSet(testKeyNSList.metadata)
require.NoError(t, iMgr.AddToList(testKeyNSList.metadata, []*IPSetMetadata{testNSSet.metadata, testKeyPodSet.metadata}))
iMgr.CreateIPSet(testKVNSList.metadata)
require.NoError(t, iMgr.AddToList(testKVNSList.metadata, []*IPSetMetadata{testKVPodSet.metadata}))
iMgr.CreateIPSet(testNestedLabelList.metadata)
toAddOrUpdateSetNames := []string{
testNSSet.metadata.GetPrefixName(),
testKeyPodSet.metadata.GetPrefixName(),
testKVPodSet.metadata.GetPrefixName(),
testNamedportSet.metadata.GetPrefixName(),
testCIDRSet.metadata.GetPrefixName(),
testKeyNSList.metadata.GetPrefixName(),
testKVNSList.metadata.GetPrefixName(),
testNestedLabelList.metadata.GetPrefixName(),
}
assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache)
creator := iMgr.getFileCreator(1, nil, toAddOrUpdateSetNames)
actualFileString := getSortedFileString(creator)
assertEqualFileStrings(t, expectedFileString, actualFileString)
wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.NoError(t, err)
require.False(t, wasFileAltered)
}
func TestApplyDeletions(t *testing.T) {
calls := []testutils.TestCmd{fakeRestoreSuccessCommand}
iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls))
// Remove members and delete others
iMgr.CreateIPSet(testNSSet.metadata)
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.0", "a"))
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.1", "b"))
iMgr.CreateIPSet(testKeyPodSet.metadata)
iMgr.CreateIPSet(testKeyNSList.metadata)
require.NoError(t, iMgr.AddToList(testKeyNSList.metadata, []*IPSetMetadata{testNSSet.metadata, testKeyPodSet.metadata}))
require.NoError(t, iMgr.RemoveFromSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.1", "b"))
require.NoError(t, iMgr.RemoveFromList(testKeyNSList.metadata, []*IPSetMetadata{testKeyPodSet.metadata}))
iMgr.CreateIPSet(testCIDRSet.metadata)
iMgr.DeleteIPSet(testCIDRSet.metadata.GetPrefixName())
iMgr.CreateIPSet(testNestedLabelList.metadata)
iMgr.DeleteIPSet(testNestedLabelList.metadata.GetPrefixName())
toDeleteSetNames := []string{testCIDRSet.metadata.GetPrefixName(), testNestedLabelList.metadata.GetPrefixName()}
assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache)
toAddOrUpdateSetNames := []string{testNSSet.metadata.GetPrefixName(), testKeyPodSet.metadata.GetPrefixName(), testKeyNSList.metadata.GetPrefixName()}
assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache)
creator := iMgr.getFileCreator(1, toDeleteSetNames, toAddOrUpdateSetNames)
actualFileString := getSortedFileString(creator)
lines := []string{
fmt.Sprintf("-F %s", testCIDRSet.hashedName),
fmt.Sprintf("-F %s", testNestedLabelList.hashedName),
fmt.Sprintf("-X %s", testCIDRSet.hashedName),
fmt.Sprintf("-X %s", testNestedLabelList.hashedName),
fmt.Sprintf("-N %s -exist nethash", testNSSet.hashedName),
fmt.Sprintf("-N %s -exist nethash", testKeyPodSet.hashedName),
fmt.Sprintf("-N %s -exist setlist", testKeyNSList.hashedName),
}
lines = append(lines, getSortedLines(testNSSet, "10.0.0.0")...)
lines = append(lines, getSortedLines(testKeyPodSet)...)
lines = append(lines, getSortedLines(testKeyNSList, testNSSet.hashedName)...)
expectedFileString := strings.Join(lines, "\n") + "\n"
assertEqualFileStrings(t, expectedFileString, actualFileString)
wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.NoError(t, err)
require.False(t, wasFileAltered)
}
// TODO test that a reconcile list is updated
func TestFailureOnCreation(t *testing.T) {
setAlreadyExistsCommand := testutils.TestCmd{
Cmd: ipsetRestoreStringSlice,
Stdout: "Error in line 3: Set cannot be created: set with the same name already exists",
ExitCode: 1,
}
calls := []testutils.TestCmd{setAlreadyExistsCommand, fakeRestoreSuccessCommand}
iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls))
iMgr.CreateIPSet(testNSSet.metadata)
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.0", "a"))
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.1", "b"))
iMgr.CreateIPSet(testKeyPodSet.metadata)
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testKeyPodSet.metadata}, "10.0.0.5", "c"))
iMgr.CreateIPSet(testCIDRSet.metadata)
iMgr.DeleteIPSet(testCIDRSet.metadata.GetPrefixName())
toAddOrUpdateSetNames := []string{testNSSet.metadata.GetPrefixName(), testKeyPodSet.metadata.GetPrefixName()}
assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache)
toDeleteSetNames := []string{testCIDRSet.metadata.GetPrefixName()}
assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache)
creator := iMgr.getFileCreator(2, toDeleteSetNames, toAddOrUpdateSetNames)
wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.Error(t, err)
require.True(t, wasFileAltered)
lines := []string{
fmt.Sprintf("-F %s", testCIDRSet.hashedName),
fmt.Sprintf("-X %s", testCIDRSet.hashedName),
fmt.Sprintf("-N %s -exist nethash", testKeyPodSet.hashedName),
}
lines = append(lines, getSortedLines(testKeyPodSet, "10.0.0.5")...)
expectedFileString := strings.Join(lines, "\n") + "\n"
actualFileString := getSortedFileString(creator)
assertEqualFileStrings(t, expectedFileString, actualFileString)
wasFileAltered, err = creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.NoError(t, err)
require.False(t, wasFileAltered)
}
// TODO test that a reconcile list is updated
func TestFailureOnAddToList(t *testing.T) {
// This exact scenario wouldn't occur. This error happens when the cache is out of date with the kernel.
setAlreadyExistsCommand := testutils.TestCmd{
Cmd: ipsetRestoreStringSlice,
Stdout: "Error in line 12: Set to be added/deleted/tested as element does not exist",
ExitCode: 1,
}
calls := []testutils.TestCmd{setAlreadyExistsCommand, fakeRestoreSuccessCommand}
iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls))
iMgr.CreateIPSet(testNSSet.metadata)
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.0", "a"))
iMgr.CreateIPSet(testKeyPodSet.metadata)
iMgr.CreateIPSet(testKeyNSList.metadata)
require.NoError(t, iMgr.AddToList(testKeyNSList.metadata, []*IPSetMetadata{testNSSet.metadata, testKeyPodSet.metadata}))
iMgr.CreateIPSet(testKVNSList.metadata)
require.NoError(t, iMgr.AddToList(testKVNSList.metadata, []*IPSetMetadata{testNSSet.metadata}))
iMgr.CreateIPSet(testCIDRSet.metadata)
iMgr.DeleteIPSet(testCIDRSet.metadata.GetPrefixName())
toAddOrUpdateSetNames := []string{
testNSSet.metadata.GetPrefixName(),
testKeyPodSet.metadata.GetPrefixName(),
testKeyNSList.metadata.GetPrefixName(),
testKVNSList.metadata.GetPrefixName(),
}
assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache)
toDeleteSetNames := []string{testCIDRSet.metadata.GetPrefixName()}
assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache)
creator := iMgr.getFileCreator(2, toDeleteSetNames, toAddOrUpdateSetNames)
originalFileString := creator.ToString()
wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.Error(t, err)
require.True(t, wasFileAltered)
lines := []string{
fmt.Sprintf("-F %s", testCIDRSet.hashedName),
fmt.Sprintf("-X %s", testCIDRSet.hashedName),
fmt.Sprintf("-N %s -exist nethash", testNSSet.hashedName),
fmt.Sprintf("-N %s -exist nethash", testKeyPodSet.hashedName),
fmt.Sprintf("-N %s -exist setlist", testKeyNSList.hashedName),
fmt.Sprintf("-N %s -exist setlist", testKVNSList.hashedName),
}
lines = append(lines, getSortedLines(testNSSet, "10.0.0.0")...)
lines = append(lines, getSortedLines(testKeyPodSet)...) // line 9
lines = append(lines, getSortedLines(testKeyNSList, testNSSet.hashedName, testKeyPodSet.hashedName)...) // lines 10, 11, 12
lines = append(lines, getSortedLines(testKVNSList, testNSSet.hashedName)...)
expectedFileString := strings.Join(lines, "\n") + "\n"
// need this because adds are nondeterminstic
badLine := strings.Split(originalFileString, "\n")[12-1]
if badLine != fmt.Sprintf("-A %s %s", testKeyNSList.hashedName, testNSSet.hashedName) && badLine != fmt.Sprintf("-A %s %s", testKeyNSList.hashedName, testKeyPodSet.hashedName) {
require.FailNow(t, "incorrect failed line")
}
expectedFileString = strings.ReplaceAll(expectedFileString, badLine+"\n", "")
actualFileString := getSortedFileString(creator)
assertEqualFileStrings(t, expectedFileString, actualFileString)
wasFileAltered, err = creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.NoError(t, err)
require.False(t, wasFileAltered)
}
// TODO test that a reconcile list is updated
func TestFailureOnFlush(t *testing.T) {
// This exact scenario wouldn't occur. This error happens when the cache is out of date with the kernel.
setAlreadyExistsCommand := testutils.TestCmd{
Cmd: ipsetRestoreStringSlice,
Stdout: "Error in line 1: The set with the given name does not exist",
ExitCode: 1,
}
calls := []testutils.TestCmd{setAlreadyExistsCommand, fakeRestoreSuccessCommand}
iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls))
iMgr.CreateIPSet(testNSSet.metadata)
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.0", "a"))
iMgr.CreateIPSet(testKVPodSet.metadata)
iMgr.DeleteIPSet(testKVPodSet.metadata.GetPrefixName())
iMgr.CreateIPSet(testCIDRSet.metadata)
iMgr.DeleteIPSet(testCIDRSet.metadata.GetPrefixName())
toAddOrUpdateSetNames := []string{testNSSet.metadata.GetPrefixName()}
assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache)
toDeleteSetNames := []string{testKVPodSet.metadata.GetPrefixName(), testCIDRSet.metadata.GetPrefixName()}
assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache)
creator := iMgr.getFileCreator(2, toDeleteSetNames, toAddOrUpdateSetNames)
wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.Error(t, err)
require.True(t, wasFileAltered)
lines := []string{
fmt.Sprintf("-F %s", testCIDRSet.hashedName),
fmt.Sprintf("-X %s", testCIDRSet.hashedName),
fmt.Sprintf("-N %s -exist nethash", testNSSet.hashedName),
}
lines = append(lines, getSortedLines(testNSSet, "10.0.0.0")...)
expectedFileString := strings.Join(lines, "\n") + "\n"
actualFileString := getSortedFileString(creator)
assertEqualFileStrings(t, expectedFileString, actualFileString)
wasFileAltered, err = creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.NoError(t, err)
require.False(t, wasFileAltered)
}
// TODO test that a reconcile list is updated
func TestFailureOnDeletion(t *testing.T) {
setAlreadyExistsCommand := testutils.TestCmd{
Cmd: ipsetRestoreStringSlice,
Stdout: "Error in line 3: Set cannot be destroyed: it is in use by a kernel component",
ExitCode: 1,
}
calls := []testutils.TestCmd{setAlreadyExistsCommand, fakeRestoreSuccessCommand}
iMgr := NewIPSetManager(iMgrApplyAllCfg, common.NewMockIOShim(calls))
iMgr.CreateIPSet(testNSSet.metadata)
require.NoError(t, iMgr.AddToSet([]*IPSetMetadata{testNSSet.metadata}, "10.0.0.0", "a"))
iMgr.CreateIPSet(testKVPodSet.metadata)
iMgr.DeleteIPSet(testKVPodSet.metadata.GetPrefixName())
iMgr.CreateIPSet(testCIDRSet.metadata)
iMgr.DeleteIPSet(testCIDRSet.metadata.GetPrefixName())
toAddOrUpdateSetNames := []string{testNSSet.metadata.GetPrefixName()}
assertEqualContentsTestHelper(t, toAddOrUpdateSetNames, iMgr.toAddOrUpdateCache)
toDeleteSetNames := []string{testKVPodSet.metadata.GetPrefixName(), testCIDRSet.metadata.GetPrefixName()}
assertEqualContentsTestHelper(t, toDeleteSetNames, iMgr.toDeleteCache)
creator := iMgr.getFileCreator(2, toDeleteSetNames, toAddOrUpdateSetNames)
wasFileAltered, err := creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.Error(t, err)
require.True(t, wasFileAltered)
lines := []string{
fmt.Sprintf("-F %s", testKVPodSet.hashedName),
fmt.Sprintf("-F %s", testCIDRSet.hashedName),
fmt.Sprintf("-X %s", testCIDRSet.hashedName),
fmt.Sprintf("-N %s -exist nethash", testNSSet.hashedName),
}
lines = append(lines, getSortedLines(testNSSet, "10.0.0.0")...)
expectedFileString := strings.Join(lines, "\n") + "\n"
actualFileString := getSortedFileString(creator)
assertEqualFileStrings(t, expectedFileString, actualFileString)
wasFileAltered, err = creator.RunCommandOnceWithFile(util.Ipset, util.IpsetRestoreFlag)
require.NoError(t, err)
require.False(t, wasFileAltered)
}
// TODO if we add file-level error handlers, add tests for them
func assertEqualContentsTestHelper(t *testing.T, setNames []string, cache map[string]struct{}) {
require.Equal(t, len(setNames), len(cache), "cache is different than list of set names")
for _, setName := range setNames {
_, exists := cache[setName]
require.True(t, exists, "cache is different than list of set names")
}
}
// the order of adds is nondeterministic, so we're sorting them
func getSortedLines(set *testSet, members ...string) []string {
result := []string{fmt.Sprintf("-F %s", set.hashedName)}
adds := make([]string, len(members))
for k, member := range members {
adds[k] = fmt.Sprintf("-A %s %s", set.hashedName, member)
}
sort.Strings(adds)
return append(result, adds...)
}
// the order of adds is nondeterministic, so we're sorting all neighboring adds
func getSortedFileString(creator *ioutil.FileCreator) string {
lines := strings.Split(creator.ToString(), "\n")
sortedLines := make([]string, 0)
k := 0
for k < len(lines) {
line := lines[k]
if !isAddLine(line) {
sortedLines = append(sortedLines, line)
k++
continue
}
addLines := make([]string, 0)
for k < len(lines) {
line := lines[k]
if !isAddLine(line) {
break
}
addLines = append(addLines, line)
k++
}
sort.Strings(addLines)
sortedLines = append(sortedLines, addLines...)
}
return strings.Join(sortedLines, "\n")
}
func isAddLine(line string) bool {
return len(line) >= 2 && line[:2] == "-A"
}
func assertEqualFileStrings(t *testing.T, expectedFileString, actualFileString string) {
if expectedFileString == actualFileString {
return
}
fmt.Println("EXPECTED FILE STRING:")
for _, line := range strings.Split(expectedFileString, "\n") {
fmt.Println(line)
}
fmt.Println("ACTUAL FILE STRING")
for _, line := range strings.Split(actualFileString, "\n") {
fmt.Println(line)
}
require.FailNow(t, "got unexpected file string (see print contents above)")
}

Просмотреть файл

@ -19,8 +19,13 @@ const (
testPodIP = "10.0.0.0"
)
var iMgrApplyOnNeedCfg = &IPSetManagerCfg{
IPSetMode: ApplyOnNeed,
NetworkName: "azure",
}
func TestCreateIPSet(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setMetadata := NewIPSetMetadata(testSetName, NameSpace)
iMgr.CreateIPSet(setMetadata)
@ -36,7 +41,7 @@ func TestCreateIPSet(t *testing.T) {
}
func TestAddToSet(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setMetadata := NewIPSetMetadata(testSetName, NameSpace)
iMgr.CreateIPSet(setMetadata)
@ -58,7 +63,7 @@ func TestAddToSet(t *testing.T) {
}
func TestRemoveFromSet(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setMetadata := NewIPSetMetadata(testSetName, NameSpace)
iMgr.CreateIPSet(setMetadata)
@ -69,14 +74,14 @@ func TestRemoveFromSet(t *testing.T) {
}
func TestRemoveFromSetMissing(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setMetadata := NewIPSetMetadata(testSetName, NameSpace)
err := iMgr.RemoveFromSet([]*IPSetMetadata{setMetadata}, testPodIP, testPodKey)
require.Error(t, err)
}
func TestAddToListMissing(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setMetadata := NewIPSetMetadata(testSetName, NameSpace)
listMetadata := NewIPSetMetadata("testlabel", KeyLabelOfNameSpace)
err := iMgr.AddToList(listMetadata, []*IPSetMetadata{setMetadata})
@ -84,7 +89,7 @@ func TestAddToListMissing(t *testing.T) {
}
func TestAddToList(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setMetadata := NewIPSetMetadata(testSetName, NameSpace)
listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNameSpace)
iMgr.CreateIPSet(setMetadata)
@ -102,7 +107,7 @@ func TestAddToList(t *testing.T) {
}
func TestRemoveFromList(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setMetadata := NewIPSetMetadata(testSetName, NameSpace)
listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNameSpace)
iMgr.CreateIPSet(setMetadata)
@ -127,7 +132,7 @@ func TestRemoveFromList(t *testing.T) {
}
func TestRemoveFromListMissing(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setMetadata := NewIPSetMetadata(testSetName, NameSpace)
listMetadata := NewIPSetMetadata(testListName, KeyLabelOfNameSpace)
@ -138,7 +143,7 @@ func TestRemoveFromListMissing(t *testing.T) {
}
func TestDeleteIPSet(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setMetadata := NewIPSetMetadata(testSetName, NameSpace)
iMgr.CreateIPSet(setMetadata)
@ -147,7 +152,7 @@ func TestDeleteIPSet(t *testing.T) {
}
func TestGetIPsFromSelectorIPSets(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setsTocreate := []*IPSetMetadata{
{
Name: "setNs1",
@ -198,7 +203,7 @@ func TestGetIPsFromSelectorIPSets(t *testing.T) {
}
func TestAddDeleteSelectorReferences(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setsTocreate := []*IPSetMetadata{
{
Name: "setNs1",
@ -274,7 +279,7 @@ func TestAddDeleteSelectorReferences(t *testing.T) {
}
func TestAddDeleteNetPolReferences(t *testing.T) {
iMgr := NewIPSetManager("azure", common.NewMockIOShim([]testutils.TestCmd{}))
iMgr := NewIPSetManager(iMgrApplyOnNeedCfg, common.NewMockIOShim([]testutils.TestCmd{}))
setsTocreate := []*IPSetMetadata{
{
Name: "setNs1",

Просмотреть файл

@ -16,6 +16,11 @@ const (
SetPolicyTypeNestedIPSet hcn.SetPolicyType = "NESTEDIPSET"
)
func (iMgr *IPSetManager) resetIPSets() error {
// TODO
return nil
}
func (iMgr *IPSetManager) applyIPSets() error {
network, err := iMgr.getHCnNetwork()
if err != nil {

Просмотреть файл

@ -54,6 +54,7 @@ const (
DeleteSelectorReference = "DeleteSelectorReference"
AddNetPolReference = "AddNetPolReference"
DeleteNetPolReference = "DeleteNetPolReference"
RunFileCreator = "RunCommandWithFile"
)
// Error codes for ipsetmanager

Просмотреть файл

@ -0,0 +1,116 @@
package main
import (
"github.com/Azure/azure-container-networking/common"
"github.com/Azure/azure-container-networking/npm/pkg/dataplane"
"github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets"
"github.com/Azure/azure-container-networking/npm/util"
)
type testSet struct {
metadata *ipsets.IPSetMetadata
hashedName string
}
func createTestSet(name string, setType ipsets.SetType) *testSet {
set := &testSet{
metadata: &ipsets.IPSetMetadata{
Name: name,
Type: setType,
},
}
set.hashedName = util.GetHashedName(set.metadata.GetPrefixName())
return set
}
var (
testNSSet = createTestSet("test-ns-set", ipsets.NameSpace)
testKeyPodSet = createTestSet("test-keyPod-set", ipsets.KeyLabelOfPod)
testKVPodSet = createTestSet("test-kvPod-set", ipsets.KeyValueLabelOfPod)
testNamedportSet = createTestSet("test-namedport-set", ipsets.NamedPorts)
testCIDRSet = createTestSet("test-cidr-set", ipsets.CIDRBlocks)
// testKeyNSList = createTestSet("test-keyNS-list", ipsets.KeyLabelOfNameSpace)
// testKVNSList = createTestSet("test-kvNS-list", ipsets.KeyValueLabelOfNameSpace)
// testNestedLabelList = createTestSet("test-nestedlabel-list", ipsets.NestedLabelOfPod)
)
func main() {
dp := dataplane.NewDataPlane("", common.NewIOShim())
// add all types of ipsets, some with members added
dp.CreateIPSet(testNSSet.metadata)
if err := dp.AddToSet([]*ipsets.IPSetMetadata{testNSSet.metadata}, "10.0.0.0", "a"); err != nil {
panic(err)
}
if err := dp.AddToSet([]*ipsets.IPSetMetadata{testNSSet.metadata}, "10.0.0.1", "b"); err != nil {
panic(err)
}
dp.CreateIPSet(testKeyPodSet.metadata)
if err := dp.AddToSet([]*ipsets.IPSetMetadata{testKeyPodSet.metadata}, "10.0.0.5", "c"); err != nil {
panic(err)
}
dp.CreateIPSet(testKVPodSet.metadata)
dp.CreateIPSet(testNamedportSet.metadata)
dp.CreateIPSet(testCIDRSet.metadata)
// can't do lists on my computer
if err := dp.ApplyDataPlane(); err != nil {
panic(err)
}
// remove members from some sets and delete some sets
if err := dp.RemoveFromSet([]*ipsets.IPSetMetadata{testNSSet.metadata}, "10.0.0.1", "b"); err != nil {
panic(err)
}
dp.DeleteIPSet(testKVPodSet.metadata)
if err := dp.ApplyDataPlane(); err != nil {
panic(err)
}
// NOTE for Linux
/*
ipset test SETNAME ENTRYNAME:
Warning: 10.0.0.5 is in set azure-npm-2031808719.
10.0.0.4 is NOT in set azure-npm-2031808719.
ipset list (references are from setlist or iptables):
Name: azure-npm-3382169694
Type: hash:net
Revision: 6
Header: family inet hashsize 1024 maxelem 65536
Size in memory: 512
References: 0
Number of entries: 1
Members:
10.0.0.0
Name: azure-npm-2031808719
Type: hash:net
Revision: 6
Header: family inet hashsize 1024 maxelem 65536
Size in memory: 512
References: 0
Number of entries: 1
Members:
10.0.0.5
Name: azure-npm-164288419
Type: hash:ip,port
Revision: 5
Header: family inet hashsize 1024 maxelem 65536
Size in memory: 192
References: 0
Number of entries: 0
Members:
Name: azure-npm-3216600258
Type: hash:net
Revision: 6
Header: family inet hashsize 1024 maxelem 4294967295
Size in memory: 448
References: 0
Number of entries: 0
Members:
*/
}