Skip to content

Commit

Permalink
Copy Azure setup files only on Azure
Browse files Browse the repository at this point in the history
AzureCloudNodeManagerPath was being copied to all platforms, causing
errors. This change ensures AzureCloudNodeManagerPath is only copied
on Azure.
  • Loading branch information
wgahnagl authored and openshift-cherrypick-robot committed Oct 13, 2023
1 parent 79aba8d commit 3055757
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 24 deletions.
2 changes: 1 addition & 1 deletion pkg/csr/csr.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func matchesHostname(nodeName string, windowsInstances []*instance.Info,
// findHostName returns the actual host name of the instance by running the 'hostname' command
func findHostName(instanceInfo *instance.Info, instanceSigner ssh.Signer) (string, error) {
// We don't need to pass most args here as we just need to be able to run commands on the instance.
win, err := windows.New("", instanceInfo, instanceSigner)
win, err := windows.New("", instanceInfo, instanceSigner, nil)
if err != nil {
return "", fmt.Errorf("error instantiating Windows instance: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/nodeconfig/nodeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func NewNodeConfig(c client.Client, clientset *kubernetes.Clientset, clusterServ
}

log := ctrl.Log.WithName(fmt.Sprintf("nc %s", instanceInfo.Address))
win, err := windows.New(clusterDNS, instanceInfo, signer)
win, err := windows.New(clusterDNS, instanceInfo, signer, &platformType)
if err != nil {
return nil, fmt.Errorf("error instantiating Windows instance from VM: %w", err)
}
Expand Down
52 changes: 30 additions & 22 deletions pkg/windows/windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strings"

"github.com/go-logr/logr"
config "github.com/openshift/api/config/v1"
"golang.org/x/crypto/ssh"
"k8s.io/apimachinery/pkg/util/wait"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -137,8 +138,6 @@ const (
)

var (
// filesToTransfer is a map of what files should be copied to the Windows VM and where they should be copied to
filesToTransfer map[*payload.FileInfo]string
// RequiredServices is a list of Windows services installed by WMCO. WICD owns all services aside from itself.
// The order of this slice matters due to service dependencies. If a service depends on another service, the
// dependent service should be placed before the service it depends on.
Expand Down Expand Up @@ -167,11 +166,22 @@ var (
}
)

// getFilesToTransfer returns the properly populated filesToTransfer map. Note this does not include the WICD binary.
func getFilesToTransfer() (map[*payload.FileInfo]string, error) {
if filesToTransfer != nil {
return filesToTransfer, nil
// createPayload returns the map of files to transfer with generated file info
func createPayload(platform *config.PlatformType) (map[*payload.FileInfo]string, error) {
srcDestPairs := getFilesToTransfer(platform)
files := make(map[*payload.FileInfo]string)
for src, dest := range srcDestPairs {
f, err := payload.NewFileInfo(src)
if err != nil {
return nil, fmt.Errorf("could not create FileInfo object for file %s: %w", src, err)
}
files[f] = dest
}
return files, nil
}

// getFilesToTransfer returns the properly populated filesToTransfer map. Note this does not include the WICD binary.
func getFilesToTransfer(platform *config.PlatformType) map[string]string {
srcDestPairs := map[string]string{
payload.GcpGetValidHostnameScriptPath: remoteDir,
payload.WinDefenderExclusionScriptPath: remoteDir,
Expand All @@ -183,24 +193,18 @@ func getFilesToTransfer() (map[*payload.FileInfo]string, error) {
payload.WinOverlayCNIPlugin: cniDir,
payload.KubeProxyPath: K8sDir,
payload.KubeletPath: K8sDir,
payload.AzureCloudNodeManagerPath: K8sDir,
payload.KubeLogRunnerPath: K8sDir,
payload.CSIProxyPath: K8sDir,
payload.ContainerdPath: ContainerdDir,
payload.HcsshimPath: ContainerdDir,
payload.ContainerdConfPath: ContainerdDir,
payload.NetworkConfigurationScript: remoteDir,
}
files := make(map[*payload.FileInfo]string)
for src, dest := range srcDestPairs {
f, err := payload.NewFileInfo(src)
if err != nil {
return nil, fmt.Errorf("could not create FileInfo object for file %s: %w", src, err)
}
files[f] = dest

if platform != nil && *platform == config.AzurePlatformType {
srcDestPairs[payload.AzureCloudNodeManagerPath] = K8sDir
}
filesToTransfer = files
return filesToTransfer, nil
return srcDestPairs
}

// GetK8sDir returns the location of the kubernetes executable directory
Expand Down Expand Up @@ -257,23 +261,31 @@ type windows struct {
log logr.Logger
// defaultShellPowerShell indicates if the default SSH shell is PowerShell
defaultShellPowerShell bool
// filesToTransfer is the map of files needed for the windows VM
filesToTransfer map[*payload.FileInfo]string
}

// New returns a new Windows instance constructed from the given WindowsVM
func New(clusterDNS string, instanceInfo *instance.Info, signer ssh.Signer) (Windows, error) {
func New(clusterDNS string, instanceInfo *instance.Info, signer ssh.Signer, platform *config.PlatformType) (Windows, error) {
log := ctrl.Log.WithName(fmt.Sprintf("wc %s", instanceInfo.Address))
log.V(1).Info("initializing SSH connection")
conn, err := newSshConnectivity(instanceInfo.Username, instanceInfo.Address, signer, log)
if err != nil {
return nil, fmt.Errorf("unable to setup VM %s sshConnectivity: %w", instanceInfo.Address, err)
}

files, err := createPayload(platform)
if err != nil {
return nil, fmt.Errorf("unable to create payload: %w", err)
}

return &windows{
interact: conn,
clusterDNS: clusterDNS,
instance: instanceInfo,
log: log,
defaultShellPowerShell: defaultShellPowershell(conn),
filesToTransfer: files,
},
nil
}
Expand Down Expand Up @@ -652,11 +664,7 @@ func (vm *windows) removeDirectories() error {
// transferFiles copies various files required for configuring the Windows node, to the VM.
func (vm *windows) transferFiles() error {
vm.log.Info("transferring files")
filesToTransfer, err := getFilesToTransfer()
if err != nil {
return fmt.Errorf("error getting list of files to transfer: %w", err)
}
for src, dest := range filesToTransfer {
for src, dest := range vm.filesToTransfer {
if err := vm.EnsureFile(src, dest); err != nil {
return fmt.Errorf("error copying %s to %s: %w", src.Path, dest, err)
}
Expand Down
36 changes: 36 additions & 0 deletions pkg/windows/windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,45 @@ package windows
import (
"testing"

config "github.com/openshift/api/config/v1"
"github.com/stretchr/testify/assert"

"github.com/openshift/windows-machine-config-operator/pkg/nodeconfig/payload"
)

func TestGetFilesToTransfer(t *testing.T) {
testCases := []struct {
name string
platform *config.PlatformType
}{
{
name: "test AWS",
platform: func() *config.PlatformType { t := config.AWSPlatformType; return &t }(),
},
{
name: "test Azure",
platform: func() *config.PlatformType { t := config.AzurePlatformType; return &t }(),
},
{
name: "test Nil",
platform: nil,
},
}

for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
files := getFilesToTransfer(test.platform)
if test.platform != nil && *test.platform == config.AzurePlatformType {
file := files[payload.AzureCloudNodeManagerPath]
assert.Equal(t, K8sDir, file)
} else {
_, exists := files[payload.AzureCloudNodeManagerPath]
assert.False(t, exists)
}
})
}
}

func TestSplitPath(t *testing.T) {
testCases := []struct {
name string
Expand Down

0 comments on commit 3055757

Please sign in to comment.