Skip to content

Commit

Permalink
Merge pull request openshift#1897 from openshift-cherrypick-robot/che…
Browse files Browse the repository at this point in the history
…rry-pick-1778-to-release-4.14

[release-4.14] OCPBUGS-20664: Copy Azure setup files only on Azure
  • Loading branch information
openshift-ci[bot] committed Oct 16, 2023
2 parents 652db80 + 3055757 commit eabefc4
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 eabefc4

Please sign in to comment.