Repository: Azure/ARO-RP
Pull request #1767 (2021-09-29 07:09:07)

change order to ensure startVM is done early
### Which issue this PR addresses:

Fixes case when PUCM is attempted on a cluster when VMs are all stopped.

### What this PR does / why we need it:

At the moment, when all VMs are stopped, PUCM  fails as API server is accessed in a recently added step before startVM. This PR fixes the order by putting the startVM before.

### Test plan for issue:

Will rely on CI, E2E

### Is there any documentation that needs to be updated for this PR?

N/A



Patch:
diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go
index aedcd7212..e0fa4746f 100644
--- a/pkg/cluster/install.go
+++ b/pkg/cluster/install.go
@@ -35,9 +35,9 @@ func (m *manager) AdminUpdate(ctx context.Context) error {
 		steps.Action(m.ensureDefaults),
 		steps.AuthorizationRefreshingAction(m.fpAuthorizer, steps.Action(m.ensureResourceGroup)), // re-create RP RBAC if needed after tenant migration
 		steps.Action(m.createOrUpdateDenyAssignment),
-		steps.Action(m.populateRegistryStorageAccountName),
 		steps.Action(m.startVMs),
 		steps.Condition(m.apiServersReady, 30*time.Minute, false),
+		steps.Action(m.populateRegistryStorageAccountName),
 		steps.Action(m.ensureBillingRecord), // belt and braces
 		steps.Action(m.configureAPIServerCertificate),
 		steps.Action(m.configureIngressCertificate),
diff --git a/pkg/util/log/audit/audit.go b/pkg/util/log/audit/audit.go
index 4b17b53f7..b85c0efd8 100644
--- a/pkg/util/log/audit/audit.go
+++ b/pkg/util/log/audit/audit.go
@@ -5,7 +5,6 @@ package audit
 
 import (
 	"encoding/json"
-	"fmt"
 	"sync"
 
 	"github.com/gofrs/uuid"
@@ -70,118 +69,125 @@ var (
 	seqNumMutex sync.Mutex
 )
 
-// PayloadHook, when fires, hydrates an IFxAudit log payload using data in a log
+// AddHook modifies logger by adding the payload hook to its list of hooks.
+func AddHook(logger *logrus.Logger) {
+	logger.AddHook(&payloadHook{
+		payload: &Payload{},
+	})
+}
+
+// payloadHook, when fires, hydrates an IFxAudit log payload using data in a log
 // entry.
-type PayloadHook struct {
-	Payload *Payload
+type payloadHook struct {
+	payload *Payload
 }
 
-func (PayloadHook) Levels() []logrus.Level {
+func (payloadHook) Levels() []logrus.Level {
 	return logrus.AllLevels
 }
 
-func (h *PayloadHook) Fire(entry *logrus.Entry) error {
-	h.Payload = &Payload{}
+func (h *payloadHook) Fire(entry *logrus.Entry) error {
+	h.payload = &Payload{}
 
 	// Part-A
-	h.Payload.EnvVer = IFXAuditVersion
-	h.Payload.EnvName = IFXAuditName
+	h.payload.EnvVer = IFXAuditVersion
+	h.payload.EnvName = IFXAuditName
 
 	if v, ok := entry.Data[MetadataCreatedTime].(string); ok {
-		h.Payload.EnvTime = v
+		h.payload.EnvTime = v
 	}
 
-	h.Payload.EnvEpoch = epoch
-	h.Payload.EnvSeqNum = nextSeqNum()
+	h.payload.EnvEpoch = epoch
+	h.payload.EnvSeqNum = nextSeqNum()
 
-	if v, ok := entry.Data[EnvKeyIKey]; ok {
-		h.Payload.EnvIKey = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyIKey].(string); ok {
+		h.payload.EnvIKey = v
 		delete(entry.Data, EnvKeyIKey)
 	}
 
-	h.Payload.EnvFlags = ifxAuditFlags
+	h.payload.EnvFlags = ifxAuditFlags
 
-	if v, ok := entry.Data[EnvKeyAppID]; ok {
-		h.Payload.EnvAppID = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyAppID].(string); ok {
+		h.payload.EnvAppID = v
 		delete(entry.Data, EnvKeyAppID)
 	}
 
-	if v, ok := entry.Data[EnvKeyAppVer]; ok {
-		h.Payload.EnvAppVer = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyAppVer].(string); ok {
+		h.payload.EnvAppVer = v
 		delete(entry.Data, EnvKeyAppVer)
 	}
 
-	if v, ok := entry.Data[EnvKeyCorrelationID]; ok {
-		h.Payload.EnvCV = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyCorrelationID].(string); ok {
+		h.payload.EnvCV = v
 		delete(entry.Data, EnvKeyCorrelationID)
 	}
 
-	if v, ok := entry.Data[EnvKeyEnvironment]; ok {
-		h.Payload.EnvCloudName = fmt.Sprint(v)
-		h.Payload.EnvCloudEnvironment = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyEnvironment].(string); ok {
+		h.payload.EnvCloudName = v
+		h.payload.EnvCloudEnvironment = v
 		delete(entry.Data, EnvKeyEnvironment)
 	}
 
-	if v, ok := entry.Data[EnvKeyCloudRole]; ok {
-		h.Payload.EnvCloudRole = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyCloudRole].(string); ok {
+		h.payload.EnvCloudRole = v
 		delete(entry.Data, EnvKeyCloudRole)
 	}
 
-	if v, ok := entry.Data[EnvKeyCloudRoleVer]; ok {
-		h.Payload.EnvCloudRoleVer = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyCloudRoleVer].(string); ok {
+		h.payload.EnvCloudRoleVer = v
 		delete(entry.Data, EnvKeyCloudRoleVer)
 	}
 
-	if v, ok := entry.Data[EnvKeyHostname]; ok {
-		h.Payload.EnvCloudRoleInstance = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyHostname].(string); ok {
+		h.payload.EnvCloudRoleInstance = v
 		delete(entry.Data, EnvKeyHostname)
 	}
 
-	if v, ok := entry.Data[EnvKeyLocation]; ok {
-		h.Payload.EnvCloudLocation = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyLocation].(string); ok {
+		h.payload.EnvCloudLocation = v
 		delete(entry.Data, EnvKeyLocation)
 	}
 
-	if v, ok := entry.Data[EnvKeyCloudDeploymentUnit]; ok {
-		h.Payload.EnvCloudDeploymentUnit = fmt.Sprint(v)
+	if v, ok := entry.Data[EnvKeyCloudDeploymentUnit].(string); ok {
+		h.payload.EnvCloudDeploymentUnit = v
 		delete(entry.Data, EnvKeyCloudDeploymentUnit)
 	}
 
-	h.Payload.EnvCloudVer = IFXAuditCloudVer
+	h.payload.EnvCloudVer = IFXAuditCloudVer
 
 	// Part-B
 	if ids, ok := entry.Data[PayloadKeyCallerIdentities].([]CallerIdentity); ok {
-		h.Payload.CallerIdentities = append(h.Payload.CallerIdentities, ids...)
+		h.payload.CallerIdentities = append(h.payload.CallerIdentities, ids...)
 		delete(entry.Data, PayloadKeyCallerIdentities)
 	}
 
-	if v, ok := entry.Data[PayloadKeyCategory]; ok {
-		h.Payload.Category = fmt.Sprint(v)
+	if v, ok := entry.Data[PayloadKeyCategory].(string); ok {
+		h.payload.Category = v
 		delete(entry.Data, PayloadKeyCategory)
 	}
 
-	if v, ok := entry.Data[PayloadKeyOperationName]; ok {
-		h.Payload.OperationName = fmt.Sprint(v)
+	if v, ok := entry.Data[PayloadKeyOperationName].(string); ok {
+		h.payload.OperationName = v
 		delete(entry.Data, PayloadKeyOperationName)
 	}
 
 	if v, ok := entry.Data[PayloadKeyResult].(Result); ok {
-		h.Payload.Result = v
+		h.payload.Result = v
 		delete(entry.Data, PayloadKeyResult)
 	}
 
-	if v, ok := entry.Data[PayloadKeyRequestID]; ok {
-		h.Payload.RequestID = fmt.Sprint(v)
+	if v, ok := entry.Data[PayloadKeyRequestID].(string); ok {
+		h.payload.RequestID = v
 		delete(entry.Data, PayloadKeyRequestID)
 	}
 
 	if rs, ok := entry.Data[PayloadKeyTargetResources].([]TargetResource); ok {
-		h.Payload.TargetResources = append(h.Payload.TargetResources, rs...)
+		h.payload.TargetResources = append(h.payload.TargetResources, rs...)
 		delete(entry.Data, PayloadKeyTargetResources)
 	}
 
 	// add the audit payload
-	b, err := json.Marshal(h.Payload)
+	b, err := json.Marshal(h.payload)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/util/log/log.go b/pkg/util/log/log.go
index 112563e88..2a47ee109 100644
--- a/pkg/util/log/log.go
+++ b/pkg/util/log/log.go
@@ -45,31 +45,17 @@ func getBaseLogger() *logrus.Logger {
 		FullTimestamp: true,
 	})
 
-	logger.AddHook(&logrHook{})
+	if journal.Enabled() {
+		logger.AddHook(&journaldHook{})
+	}
 
 	return logger
 }
 
-// Important: Logger hooks loading order is important as they are executed in
-// such order (https://github.com/sirupsen/logrus/blob/master/hooks.go#L26-L34).
-//  This means we need to load all populating hooks (logrHook, auditHook)
-// first, and emitting hooks (journald) last. Otherwise enriched data is lost.
-// In addition to that we need to understand that in situations like this log output
-// in CMD might be not the same as in journald, because we emit directly to journald.
-// If to remove this, we would need additional layer for log parsing.
-
 // GetAuditEntry returns a consistently configured audit log entry
 func GetAuditEntry() *logrus.Entry {
 	auditLogger := getBaseLogger()
-
-	auditLogger.AddHook(&audit.PayloadHook{
-		Payload: &audit.Payload{},
-	})
-
-	if journal.Enabled() {
-		auditLogger.AddHook(&journaldHook{})
-	}
-
+	audit.AddHook(auditLogger)
 	return logrus.NewEntry(auditLogger)
 }
 
@@ -83,9 +69,7 @@ func GetLogger() *logrus.Entry {
 		CallerPrettyfier: relativeFilePathPrettier,
 	})
 
-	if journal.Enabled() {
-		logger.AddHook(&journaldHook{})
-	}
+	logger.AddHook(&logrHook{})
 
 	log := logrus.NewEntry(logger)
 


Test patch:
diff --git a/test/util/log/log.go b/test/util/log/log.go
index bebc3768f..55a5a4dde 100644
--- a/test/util/log/log.go
+++ b/test/util/log/log.go
@@ -29,9 +29,7 @@ func New() (*test.Hook, *logrus.Entry) {
 // feature.
 func NewAudit() (*test.Hook, *logrus.Entry) {
 	logger, h := test.NewNullLogger()
-	logger.AddHook(&audit.PayloadHook{
-		Payload: &audit.Payload{},
-	})
+	audit.AddHook(logger)
 	return h, logrus.NewEntry(logger)
 }