Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions cns/cnireconciler/podinfoprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
41 changes: 41 additions & 0 deletions cns/cnireconciler/statefile.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package cnireconciler

import (
"encoding/json"
"errors"
"os"

"github.com/Azure/azure-container-networking/cns/logger"
)

// 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"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @rbtr 👋 what's the best way I can make this different for OS windows?

I was thinking about making a statefile_windows.go with this same code but different filename, that seems to copy alot however.

Any ideas? I just want to defer to you since you know alot more about go than I do, I can only think about an if statement if the OS is windows, just wondering if that's enough

Copy link
Collaborator Author

@rbtr rbtr Oct 5, 2021

Choose a reason for hiding this comment

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

This was a hack for older CNIs, pre 1.4.7 or so; if you're adding support to Windows now then this might not even be necessary.
If it is necessary, is the only thing that's different on Windows the filepath? That could be injected from an early OS check all the way down in to this function. OR, probably more correctly, this shouldn't be a hardcoded const and should come from env or config (with a default for each OS).

return writeObjectToFile(filename)
}

func writeObjectToFile(filename string) error {
fi, err := os.Stat(filename)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return nil
Copy link
Member

Choose a reason for hiding this comment

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

Add a log line?

}
return err
}

if fi.Size() != 0 {
return nil
}

logger.Printf("Writing {} to CNI statefile")
b, _ := json.Marshal(map[string]string{})
return os.WriteFile(filename, b, os.FileMode(0666))
}
41 changes: 41 additions & 0 deletions cns/cnireconciler/statefile_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
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.MkdirAll(path.Dir(name), 0666)
require.NoError(t, err)

_, 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), "{}")
}

func init() {
logger.InitLogger("testlogs", 0, 0, "./")
}
8 changes: 8 additions & 0 deletions cns/service/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand Down