Skip to content

MGMT-13657: Don't wait for console if it is disabled - ACM 2.6 #634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 1 commit into from
Feb 14, 2023
Merged

MGMT-13657: Don't wait for console if it is disabled - ACM 2.6 #634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 19 additions & 4 deletions src/assisted_installer_controller/assisted_installer_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (

"github.com/hashicorp/go-version"
metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
configv1 "github.com/openshift/api/config/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
certificatesv1 "k8s.io/api/certificates/v1"
Expand Down Expand Up @@ -57,6 +58,8 @@ const (
ExitWaiting = true
customManifestsFile = "custom_manifests.json"
kubeconfigFileName = "kubeconfig-noingress"

consoleCapabilityName = configv1.ClusterVersionCapability("Console")
)

var (
Expand Down Expand Up @@ -985,12 +988,24 @@ func (c controller) waitingForClusterOperators(ctx context.Context) error {
ctxWithTimeout, cancel := context.WithTimeout(ctx, CVOMaxTimeout)
defer cancel()
isClusterVersionAvailable := func(timer *time.Timer) bool {
result := c.isOperatorAvailable(NewClusterOperatorHandler(c.kc, consoleOperatorName))

clusterOperatorHandler := NewClusterOperatorHandler(c.kc, consoleOperatorName)
clusterVersionHandler := NewClusterVersionHandler(c.kc, timer)
isConsoleEnabled, err := c.kc.IsClusterCapabilityEnabled(consoleCapabilityName)
if err != nil {
c.log.WithError(err).Error("Failed to check if console is enabled")
return false
}
var result bool
if isConsoleEnabled {
c.log.Info("Console is enabled, will wait for the console operator to be available")
result = c.isOperatorAvailable(clusterOperatorHandler)
} else {
c.log.Info("Console is disabled, will not wait for the console operator to be available")
result = true
}
if c.WaitForClusterVersion {
result = c.isOperatorAvailable(NewClusterVersionHandler(c.kc, timer)) && result
result = c.isOperatorAvailable(clusterVersionHandler) && result
}

return result
}
return utils.WaitForPredicateWithTimer(ctxWithTimeout, WaitTimeout, GeneralProgressUpdateInt, isClusterVersionAvailable)
Expand Down
48 changes: 39 additions & 9 deletions src/assisted_installer_controller/assisted_installer_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ var _ = Describe("installer HostRoleMaster role", func() {

setCvoAsAvailable := func() {
mockGetServiceOperators([]models.MonitoredOperator{{Name: cvoOperatorName, Status: models.OperatorStatusProgressing}})
mockk8sclient.EXPECT().GetClusterVersion(clusterVersionName).Return(availableClusterVersionCondition, nil).Times(1)
mockk8sclient.EXPECT().GetClusterVersion().Return(availableClusterVersionCondition, nil).Times(1)
mockbmclient.EXPECT().UpdateClusterOperator(gomock.Any(), gomock.Any(), cvoOperatorName, models.OperatorStatusAvailable, availableClusterVersionCondition.Status.Conditions[0].Message).Times(1)

mockGetServiceOperators([]models.MonitoredOperator{{Name: cvoOperatorName, Status: models.OperatorStatusAvailable}})
Expand Down Expand Up @@ -247,6 +247,10 @@ var _ = Describe("installer HostRoleMaster role", func() {
mockbmclient.EXPECT().DownloadClusterCredentials(gomock.Any(), kubeconfigFileName, gomock.Any()).Return(nil).Times(1)
}

mockAllCapabilitiesEnabled := func() {
mockk8sclient.EXPECT().IsClusterCapabilityEnabled(gomock.Any()).Return(true, nil).AnyTimes()
}

Context("Waiting for 3 nodes", func() {
It("Set ready event", func() {
// fail to connect to assisted and then succeed
Expand Down Expand Up @@ -590,6 +594,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
})

It("failure if console not available in service or failed to set status and success if available", func() {
mockAllCapabilitiesEnabled()

installing := models.ClusterStatusInstalling
mockbmclient.EXPECT().GetCluster(gomock.Any(), false).Return(&models.Cluster{Status: &installing}, nil).Times(2)

Expand Down Expand Up @@ -620,6 +626,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
})

It("success", func() {
mockAllCapabilitiesEnabled()

installing := models.ClusterStatusInstalling
mockbmclient.EXPECT().GetCluster(gomock.Any(), false).Return(&models.Cluster{Status: &installing}, nil).Times(1)
setControllerWaitForOLMOperators(assistedController.ClusterID)
Expand All @@ -642,6 +650,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
})

It("lots of failures then success", func() {
mockAllCapabilitiesEnabled()

installing := models.ClusterStatusInstalling
mockbmclient.EXPECT().GetCluster(gomock.Any(), false).Return(&models.Cluster{Status: &installing}, nil).Times(1)
setClusterAsFinalizing()
Expand Down Expand Up @@ -708,18 +718,18 @@ var _ = Describe("installer HostRoleMaster role", func() {

// CVO errors
mockGetServiceOperators([]models.MonitoredOperator{{Name: cvoOperatorName, Status: ""}})
mockk8sclient.EXPECT().GetClusterVersion(clusterVersionName).Return(nil, fmt.Errorf("dummy")).Times(1)
mockk8sclient.EXPECT().GetClusterVersion().Return(nil, fmt.Errorf("dummy")).Times(1)

mockGetServiceOperators([]models.MonitoredOperator{{Name: cvoOperatorName, Status: ""}})
mockk8sclient.EXPECT().GetClusterVersion(clusterVersionName).Return(progressClusterVersionCondition, nil).Times(1)
mockk8sclient.EXPECT().GetClusterVersion().Return(progressClusterVersionCondition, nil).Times(1)
mockbmclient.EXPECT().UpdateClusterOperator(gomock.Any(), gomock.Any(), cvoOperatorName, models.OperatorStatusProgressing, progressClusterVersionCondition.Status.Conditions[0].Message).Times(1)

// Fail 8 more times when console fail
extraFailTimes := 8
for i := 0; i < extraFailTimes; i++ {
mockGetServiceOperators([]models.MonitoredOperator{{Name: cvoOperatorName, Status: ""}})
}
mockk8sclient.EXPECT().GetClusterVersion(clusterVersionName).Return(nil, fmt.Errorf("dummy")).Times(extraFailTimes)
mockk8sclient.EXPECT().GetClusterVersion().Return(nil, fmt.Errorf("dummy")).Times(extraFailTimes)

setCvoAsAvailable()

Expand All @@ -739,14 +749,16 @@ var _ = Describe("installer HostRoleMaster role", func() {
})

It("failure", func() {
mockAllCapabilitiesEnabled()

setClusterAsFinalizing()

mockbmclient.EXPECT().GetClusterMonitoredOperator(gomock.Any(), gomock.Any(), consoleOperatorName, gomock.Any()).
Return(&models.MonitoredOperator{Status: "", StatusInfo: ""}, nil).AnyTimes()
mockk8sclient.EXPECT().GetClusterOperator(consoleOperatorName).Return(nil, fmt.Errorf("dummy")).AnyTimes()
mockbmclient.EXPECT().GetClusterMonitoredOperator(gomock.Any(), gomock.Any(), cvoOperatorName, gomock.Any()).
Return(&models.MonitoredOperator{Status: "", StatusInfo: ""}, nil).AnyTimes()
mockk8sclient.EXPECT().GetClusterVersion(clusterVersionName).Return(nil, fmt.Errorf("dummy")).AnyTimes()
mockk8sclient.EXPECT().GetClusterVersion().Return(nil, fmt.Errorf("dummy")).AnyTimes()

mockbmclient.EXPECT().CompleteInstallation(gomock.Any(), "cluster-id", false, gomock.Any()).Return(nil).Times(1)

Expand All @@ -763,6 +775,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
GeneralWaitInterval = 10 * time.Millisecond
})
It("success", func() {
mockAllCapabilitiesEnabled()

installing := models.ClusterStatusInstalling
mockbmclient.EXPECT().GetCluster(gomock.Any(), false).Return(&models.Cluster{Status: &installing}, nil).Times(1)
setControllerWaitForOLMOperators(assistedController.ClusterID)
Expand All @@ -780,6 +794,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
Expect(assistedController.Status.HasError()).Should(Equal(false))
})
It("failure", func() {
mockAllCapabilitiesEnabled()

setClusterAsFinalizing()
setConsoleAsAvailable("cluster-id")
mockk8sclient.EXPECT().GetConfigMap(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("aaa")).MinTimes(1)
Expand All @@ -804,6 +820,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
})

It("waiting for single OLM operator", func() {
mockAllCapabilitiesEnabled()

By("setup", func() {
setControllerWaitForOLMOperators(assistedController.ClusterID)
operators := []models.MonitoredOperator{
Expand Down Expand Up @@ -857,6 +875,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
})

It("waiting for single OLM operator which timeouts", func() {
mockAllCapabilitiesEnabled()

By("setup", func() {
setControllerWaitForOLMOperators(assistedController.ClusterID)
operators := []models.MonitoredOperator{{SubscriptionName: "local-storage-operator", Namespace: "openshift-local-storage", OperatorType: models.OperatorTypeOlm, Name: "lso", Status: models.OperatorStatusProgressing, TimeoutSeconds: 0}}
Expand Down Expand Up @@ -892,6 +912,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
GeneralWaitInterval = 10 * time.Millisecond
})
It("success", func() {
mockAllCapabilitiesEnabled()

installing := models.ClusterStatusInstalling
mockbmclient.EXPECT().GetCluster(gomock.Any(), false).Return(&models.Cluster{Status: &installing}, nil).Times(1)
setControllerWaitForOLMOperators(assistedController.ClusterID)
Expand Down Expand Up @@ -1505,6 +1527,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
for i := range tests {
t := tests[i]
It(t.name, func() {
mockAllCapabilitiesEnabled()

clusterVersionReport := &configv1.ClusterVersion{
Status: configv1.ClusterVersionStatus{
Conditions: []configv1.ClusterOperatorStatusCondition{t.newCVOCondition},
Expand All @@ -1529,7 +1553,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
if t.currentServiceCVOStatus.Status != models.OperatorStatusAvailable {
amountOfSamples++
}
mockk8sclient.EXPECT().GetClusterVersion(clusterVersionName).Return(clusterVersionReport, nil).MinTimes(amountOfSamples)
mockk8sclient.EXPECT().GetClusterVersion().Return(clusterVersionReport, nil).MinTimes(amountOfSamples)

if newServiceCVOStatus.Status == models.OperatorStatusAvailable {
Expect(assistedController.waitingForClusterOperators(ctx)).ShouldNot(HaveOccurred())
Expand All @@ -1540,6 +1564,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
}

It("service fail to sync - context cancel", func() {
mockAllCapabilitiesEnabled()

currentServiceCVOStatus := &models.MonitoredOperator{Status: models.OperatorStatusProgressing, StatusInfo: ""}
clusterVersionReport := &configv1.ClusterVersion{
Status: configv1.ClusterVersionStatus{
Expand All @@ -1549,7 +1575,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
},
}

mockk8sclient.EXPECT().GetClusterVersion(clusterVersionName).Return(clusterVersionReport, nil).AnyTimes()
mockk8sclient.EXPECT().GetClusterVersion().Return(clusterVersionReport, nil).AnyTimes()
mockbmclient.EXPECT().GetClusterMonitoredOperator(gomock.Any(), gomock.Any(), cvoOperatorName, gomock.Any()).Return(currentServiceCVOStatus, nil).AnyTimes()
mockbmclient.EXPECT().UpdateClusterOperator(gomock.Any(), gomock.Any(), cvoOperatorName, gomock.Any(), gomock.Any()).AnyTimes()

Expand All @@ -1563,6 +1589,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
})

It("service fail to sync - maxTimeout applied", func() {
mockAllCapabilitiesEnabled()

WaitTimeout = 1 * time.Second
CVOMaxTimeout = 200 * time.Millisecond
currentServiceCVOStatus := &models.MonitoredOperator{Status: models.OperatorStatusProgressing, StatusInfo: ""}
Expand All @@ -1574,7 +1602,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
},
}

mockk8sclient.EXPECT().GetClusterVersion(clusterVersionName).Return(clusterVersionReport, nil).AnyTimes()
mockk8sclient.EXPECT().GetClusterVersion().Return(clusterVersionReport, nil).AnyTimes()
mockbmclient.EXPECT().GetClusterMonitoredOperator(gomock.Any(), gomock.Any(), cvoOperatorName, gomock.Any()).Return(currentServiceCVOStatus, nil).AnyTimes()
mockbmclient.EXPECT().UpdateClusterOperator(gomock.Any(), gomock.Any(), cvoOperatorName, gomock.Any(), gomock.Any()).AnyTimes()

Expand All @@ -1586,6 +1614,8 @@ var _ = Describe("installer HostRoleMaster role", func() {
})

It("service fail to sync - finally succeed", func() {
mockAllCapabilitiesEnabled()

currentServiceCVOStatus := &models.MonitoredOperator{Status: models.OperatorStatusProgressing, StatusInfo: ""}
newServiceCVOStatus := &models.MonitoredOperator{Status: models.OperatorStatusAvailable, StatusInfo: ""}
clusterVersionReport := &configv1.ClusterVersion{
Expand All @@ -1597,7 +1627,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
}

// Fail twice
mockk8sclient.EXPECT().GetClusterVersion(clusterVersionName).Return(clusterVersionReport, nil).Times(2)
mockk8sclient.EXPECT().GetClusterVersion().Return(clusterVersionReport, nil).Times(2)
mockbmclient.EXPECT().GetClusterMonitoredOperator(gomock.Any(), gomock.Any(), cvoOperatorName, gomock.Any()).Return(currentServiceCVOStatus, nil).Times(2)
mockbmclient.EXPECT().UpdateClusterOperator(gomock.Any(), gomock.Any(), cvoOperatorName, gomock.Any(), gomock.Any()).Times(2)

Expand Down
7 changes: 3 additions & 4 deletions src/assisted_installer_controller/operator_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import (
)

const (
cvoOperatorName = "cvo"
clusterVersionName = "version"
cvoOperatorName = "cvo"
)

type OperatorHandler interface {
Expand All @@ -36,7 +35,7 @@ func (c controller) isOperatorAvailable(handler OperatorHandler) bool {
return false
}

if operatorStatusInService.Status != operatorStatus || (operatorStatusInService.StatusInfo != operatorMessage && operatorMessage != "") {
if operatorStatusInService != nil && (operatorStatusInService.Status != operatorStatus || (operatorStatusInService.StatusInfo != operatorMessage && operatorMessage != "")) {
c.log.Infof("Operator <%s> updated, status: %s -> %s, message: %s -> %s.", operatorName, operatorStatusInService.Status, operatorStatus, operatorStatusInService.StatusInfo, operatorMessage)
if !handler.OnChange(operatorStatus) {
c.log.WithError(err).Warnf("<%s> operator's OnChange() returned false. Will skip an update.", operatorName)
Expand Down Expand Up @@ -107,7 +106,7 @@ func (handler ClusterVersionHandler) GetName() string { return cvoOperatorName }
func (handler ClusterVersionHandler) IsInitialized() bool { return true }

func (handler ClusterVersionHandler) GetStatus() (models.OperatorStatus, string, error) {
co, err := handler.kc.GetClusterVersion(clusterVersionName)
co, err := handler.kc.GetClusterVersion()
if err != nil {
return "", "", err
}
Expand Down
21 changes: 16 additions & 5 deletions src/k8s_client/k8s_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import (
"strings"
"time"

"gopkg.in/yaml.v2"

metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
configv1 "github.com/openshift/api/config/v1"
configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
Expand All @@ -22,6 +20,8 @@ import (
olmv1client "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/typed/operators/v1alpha1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/thoas/go-funk"
"gopkg.in/yaml.v2"
certificatesv1 "k8s.io/api/certificates/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -69,7 +69,7 @@ type K8SClient interface {
UpdateBMHStatus(bmh *metal3v1alpha1.BareMetalHost) error
UpdateBMH(bmh *metal3v1alpha1.BareMetalHost) error
SetProxyEnvVars() error
GetClusterVersion(name string) (*configv1.ClusterVersion, error)
GetClusterVersion() (*configv1.ClusterVersion, error)
GetNetworkType() (string, error)
GetServiceNetworks() ([]string, error)
GetControlPlaneReplicas() (int, error)
Expand All @@ -83,6 +83,7 @@ type K8SClient interface {
PatchNamespace(namespace string, data []byte) error
GetNode(name string) (*v1.Node, error)
PatchNodeLabels(nodeName string, nodeLabels string) error
IsClusterCapabilityEnabled(configv1.ClusterVersionCapability) (bool, error)
}

type K8SClientBuilder func(configPath string, logger logrus.FieldLogger) (K8SClient, error)
Expand Down Expand Up @@ -551,12 +552,12 @@ func (c *k8sClient) UpdateBMH(bmh *metal3v1alpha1.BareMetalHost) error {
return c.runtimeClient.Update(context.TODO(), bmh)
}

func (c *k8sClient) GetClusterVersion(name string) (*configv1.ClusterVersion, error) {
func (c *k8sClient) GetClusterVersion() (*configv1.ClusterVersion, error) {
result := &configv1.ClusterVersion{}
err := c.client.RESTClient().Get().
AbsPath("/apis/config.openshift.io/v1").
Resource("clusterversions").
Name(name).
Name("version").
Do(context.Background()).
Into(result)
return result, err
Expand Down Expand Up @@ -613,3 +614,13 @@ func (c *k8sClient) PatchNodeLabels(nodeName string, nodeLabels string) error {
_, err := c.client.CoreV1().Nodes().Patch(context.Background(), nodeName, types.MergePatchType, data, metav1.PatchOptions{})
return err
}

func (c *k8sClient) IsClusterCapabilityEnabled(capability configv1.ClusterVersionCapability) (bool, error) {
version, err := c.GetClusterVersion()
if err != nil {
return false, err
}
isExplicitlyEnabled := funk.Contains(version.Status.Capabilities.EnabledCapabilities, capability)
isKnown := funk.Contains(version.Status.Capabilities.KnownCapabilities, capability)
return isExplicitlyEnabled || !isKnown, nil
}
23 changes: 19 additions & 4 deletions src/k8s_client/mock_k8s_client.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading