From 37233bf9a2e529d9d36bc29fa9f74eb54f1dc0d0 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Tue, 13 Jul 2021 17:17:12 -0500 Subject: [PATCH 1/2] write empty json object to empty cni statefile if exists --- cns/cnireconciler/statefile.go | 38 +++++++++++++++++++++++++++++ cns/cnireconciler/statefile_test.go | 32 ++++++++++++++++++++++++ cns/service/main.go | 8 ++++++ 3 files changed, 78 insertions(+) create mode 100644 cns/cnireconciler/statefile.go create mode 100644 cns/cnireconciler/statefile_test.go diff --git a/cns/cnireconciler/statefile.go b/cns/cnireconciler/statefile.go new file mode 100644 index 0000000000..b866c49014 --- /dev/null +++ b/cns/cnireconciler/statefile.go @@ -0,0 +1,38 @@ +package cnireconciler + +import ( + "encoding/json" + "errors" + "os" +) + +// WriteObjectToCNIStatefile checks for a file at the CNI statefile path, +// and checks if it is empty. If it is empty, writes an empty JSON object to +// it so older CNI can execute. Does nothing and returns no error if the +// file does not exist. +// +// This is a hack to get older CNI to run when CNS has mounted the statefile +// path, but the statefile wasn't written by CNI yet. Kubelet will stub an +// empty file on the host filesystem, crashing older CNI because it doesn't know +// how to handle empty statefiles. +func WriteObjectToCNIStatefile() error { + filename := "/var/run/azure-vnet.json" + return writeObjectToFile(filename) +} + +func writeObjectToFile(filename string) error { + fi, err := os.Stat(filename) + if err != nil { + if errors.Is(os.ErrNotExist, err) { + return nil + } + return err + } + + if fi.Size() != 0 { + return nil + } + + b, _ := json.Marshal(map[string]string{}) + return os.WriteFile(filename, b, os.FileMode(0666)) +} diff --git a/cns/cnireconciler/statefile_test.go b/cns/cnireconciler/statefile_test.go new file mode 100644 index 0000000000..01c94d7b9e --- /dev/null +++ b/cns/cnireconciler/statefile_test.go @@ -0,0 +1,32 @@ +package cnireconciler + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestWriteObjectToFile(t *testing.T) { + name := "testdata/test" + _, err := os.Stat(name) + require.ErrorIs(t, err, os.ErrNotExist) + + // create empty file + _, err = os.Create(name) + require.NoError(t, err) + defer os.Remove(name) + + // check it's empty + fi, _ := os.Stat(name) + assert.Equal(t, fi.Size(), int64(0)) + + // populate + require.NoError(t, writeObjectToFile(name)) + + // read + b, err := os.ReadFile(name) + require.NoError(t, err) + assert.Equal(t, string(b), "{}") +} diff --git a/cns/service/main.go b/cns/service/main.go index 17f2f03ef5..8268af0288 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -21,6 +21,7 @@ import ( "github.com/Azure/azure-container-networking/cnm/ipam" "github.com/Azure/azure-container-networking/cnm/network" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/cnireconciler" cni "github.com/Azure/azure-container-networking/cns/cnireconciler" "github.com/Azure/azure-container-networking/cns/cnsclient" "github.com/Azure/azure-container-networking/cns/common" @@ -521,6 +522,13 @@ func main() { // Initialze state in if CNS is running in CRD mode // State must be initialized before we start HTTPRestService if config.ChannelMode == cns.CRD { + // Check the CNI statefile mount, and if the file is empty + // stub an empty JSON object + if err := cnireconciler.WriteObjectToCNIStatefile(); err != nil { + logger.Errorf("Failed to write empty object to CNI state: %v", err) + return + } + // We might be configured to reinitialize state from the CNI instead of the apiserver. // If so, we should check that the the CNI is new enough to support the state commands, // otherwise we fall back to the existing behavior. From b8b0d9e572547bef249e679451f66fbb38b621eb Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Wed, 14 Jul 2021 12:46:23 -0500 Subject: [PATCH 2/2] update tests, address review comments --- cns/cnireconciler/podinfoprovider_test.go | 8 ++++++++ cns/cnireconciler/statefile.go | 5 ++++- cns/cnireconciler/statefile_test.go | 11 ++++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/cns/cnireconciler/podinfoprovider_test.go b/cns/cnireconciler/podinfoprovider_test.go index f7bd57b3a4..64149d07a0 100644 --- a/cns/cnireconciler/podinfoprovider_test.go +++ b/cns/cnireconciler/podinfoprovider_test.go @@ -36,6 +36,14 @@ func TestNewCNIPodInfoProvider(t *testing.T) { }, wantErr: false, }, + { + name: "empty CNI response", + exec: newCNIStateFakeExec( + `{}`, + ), + want: map[string]cns.PodInfo{}, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/cns/cnireconciler/statefile.go b/cns/cnireconciler/statefile.go index b866c49014..1d9b373e27 100644 --- a/cns/cnireconciler/statefile.go +++ b/cns/cnireconciler/statefile.go @@ -4,6 +4,8 @@ import ( "encoding/json" "errors" "os" + + "github.com/Azure/azure-container-networking/cns/logger" ) // WriteObjectToCNIStatefile checks for a file at the CNI statefile path, @@ -23,7 +25,7 @@ func WriteObjectToCNIStatefile() error { func writeObjectToFile(filename string) error { fi, err := os.Stat(filename) if err != nil { - if errors.Is(os.ErrNotExist, err) { + if errors.Is(err, os.ErrNotExist) { return nil } return err @@ -33,6 +35,7 @@ func writeObjectToFile(filename string) error { return nil } + logger.Printf("Writing {} to CNI statefile") b, _ := json.Marshal(map[string]string{}) return os.WriteFile(filename, b, os.FileMode(0666)) } diff --git a/cns/cnireconciler/statefile_test.go b/cns/cnireconciler/statefile_test.go index 01c94d7b9e..e53aaeca5c 100644 --- a/cns/cnireconciler/statefile_test.go +++ b/cns/cnireconciler/statefile_test.go @@ -2,15 +2,20 @@ package cnireconciler import ( "os" + "path" "testing" + "github.com/Azure/azure-container-networking/cns/logger" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestWriteObjectToFile(t *testing.T) { name := "testdata/test" - _, err := os.Stat(name) + err := os.MkdirAll(path.Dir(name), 0666) + require.NoError(t, err) + + _, err = os.Stat(name) require.ErrorIs(t, err, os.ErrNotExist) // create empty file @@ -30,3 +35,7 @@ func TestWriteObjectToFile(t *testing.T) { require.NoError(t, err) assert.Equal(t, string(b), "{}") } + +func init() { + logger.InitLogger("testlogs", 0, 0, "./") +}