Skip to content

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

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 16, 2023
Merged

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

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
97 changes: 97 additions & 0 deletions internal/bminventory/inventory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/openshift/assisted-service/internal/host/hostutil"
"github.com/openshift/assisted-service/internal/ignition"
"github.com/openshift/assisted-service/internal/infraenv"
installcfgdata "github.com/openshift/assisted-service/internal/installcfg"
installcfg "github.com/openshift/assisted-service/internal/installcfg/builder"
"github.com/openshift/assisted-service/internal/isoeditor"
"github.com/openshift/assisted-service/internal/manifests"
Expand Down Expand Up @@ -67,6 +68,7 @@ import (
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/thoas/go-funk"
"gopkg.in/yaml.v2"
"gorm.io/gorm"
"gorm.io/gorm/clause"
"k8s.io/apimachinery/pkg/apis/meta/v1/validation"
Expand Down Expand Up @@ -127,6 +129,7 @@ type Config struct {
}

const minimalOpenShiftVersionForSingleNode = "4.8.0-0.0"
const minimalOpenShiftVersionForConsoleCapability = "4.12.0-0.0"

type Interactivity bool

Expand Down Expand Up @@ -1463,11 +1466,19 @@ func (b *bareMetalInventory) UpdateClusterInstallConfigInternal(ctx context.Cont
log.WithError(err).Errorf("failed to set install config overrides feature usage for cluster %s", params.ClusterID)
}

cluster.InstallConfigOverrides = params.InstallConfigParams
err = tx.Model(&common.Cluster{}).Where(query, params.ClusterID).Update("install_config_overrides", params.InstallConfigParams).Error
if err != nil {
log.WithError(err).Errorf("failed to update install config overrides")
return nil, common.NewApiError(http.StatusInternalServerError, err)
}

err = b.updateMonitoredOperators(tx, cluster)
if err != nil {
log.WithError(err).Error("failed to update monitored operators")
return nil, common.NewApiError(http.StatusInternalServerError, err)
}

err = tx.Commit().Error
if err != nil {
log.Error(err)
Expand All @@ -1476,6 +1487,7 @@ func (b *bareMetalInventory) UpdateClusterInstallConfigInternal(ctx context.Cont
txSuccess = true
eventgen.SendInstallConfigAppliedEvent(ctx, b.eventsHandler, params.ClusterID)
log.Infof("Custom install config was applied to cluster %s", params.ClusterID)

return cluster, nil
}

Expand Down Expand Up @@ -5608,3 +5620,88 @@ func (b *bareMetalInventory) HostWithCollectedLogsExists(clusterId strfmt.UUID)
func (b *bareMetalInventory) GetKnownApprovedHosts(clusterId strfmt.UUID) ([]*common.Host, error) {
return b.hostApi.GetKnownApprovedHosts(clusterId)
}

// updateMonitoredOperators checks the content of the installer configuration and updates the list
// of monitored operators accordingly. For example, if the installer configuration uses the
// capabilities mechanism to disable the console then the console operator is removed from the list
// of monitored operators.
func (b *bareMetalInventory) updateMonitoredOperators(tx *gorm.DB, cluster *common.Cluster) error {
// Get the complete installer configuration, including the overrides:
installConfigData, err := b.installConfigBuilder.GetInstallConfig(cluster, false, "")
if err != nil {
return err
}
var installConfig installcfgdata.InstallerConfigBaremetal
err = yaml.Unmarshal(installConfigData, &installConfig)
if err != nil {
return err
}

// Since version 4.12 it is possible to disable the console via the capabilities section of
// the installer configuration. The way to do it is to set the base capability set to `None`
// and then explicitly list all the enabled capabilities.
consoleEnabled := true
logFields := logrus.Fields{
"cluster_id": cluster.ID,
"cluster_version": cluster.OpenshiftVersion,
"minimal_version": minimalOpenShiftVersionForConsoleCapability,
}
consoleCapabilitySupported, err := common.VersionGreaterOrEqual(
cluster.OpenshiftVersion,
minimalOpenShiftVersionForConsoleCapability,
)
if err != nil {
return err
}
if consoleCapabilitySupported {
capabilities := installConfig.Capabilities
if capabilities != nil {
logFields["baseline_capability_set"] = capabilities.BaselineCapabilitySet
logFields["additional_enabled_capabilities"] = capabilities.AdditionalEnabledCapabilities
if capabilities.BaselineCapabilitySet == "None" {
consoleEnabled = false
for _, capability := range capabilities.AdditionalEnabledCapabilities {
if capability == "Console" {
consoleEnabled = true
break
}
}
}
}
if consoleEnabled {
b.log.WithFields(logFields).Info(
"Console is enabled because the cluster version supports the " +
"capability and it has been explicitly enabled by " +
"the user",
)
} else {
b.log.WithFields(logFields).Info(
"Console is disabled because the cluster version supports the " +
"capability and it hasn't been explicitly enabled by " +
"the user",
)
}
} else {
consoleEnabled = true
b.log.WithFields(logFields).Info(
"Console is enabled because the cluster version doesn't support " +
"the capability",
)
}

// Add or remove the console operator to the list of monitored operators:
consoleOperator := operators.OperatorConsole
consoleOperator.ClusterID = *cluster.ID
if consoleEnabled {
b.log.WithFields(logFields).Info(
"Adding the console to the set of monitored operators",
)
err = tx.FirstOrCreate(&consoleOperator).Error
} else {
b.log.WithFields(logFields).Info(
"Removing the console from the set of monitored operators",
)
err = tx.Delete(&consoleOperator).Error
}
return err
}
121 changes: 116 additions & 5 deletions internal/bminventory/inventory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/google/uuid"
"github.com/kelseyhightower/envconfig"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
amgmtv1 "github.com/openshift-online/ocm-sdk-go/accountsmgmt/v1"
"github.com/openshift/assisted-service/internal/cluster"
Expand All @@ -45,7 +46,8 @@ import (
"github.com/openshift/assisted-service/internal/host"
"github.com/openshift/assisted-service/internal/ignition"
"github.com/openshift/assisted-service/internal/infraenv"
installcfg "github.com/openshift/assisted-service/internal/installcfg/builder"
installcfg "github.com/openshift/assisted-service/internal/installcfg"
installcfg_builder "github.com/openshift/assisted-service/internal/installcfg/builder"
"github.com/openshift/assisted-service/internal/isoeditor"
"github.com/openshift/assisted-service/internal/metrics"
"github.com/openshift/assisted-service/internal/operators"
Expand All @@ -67,6 +69,7 @@ import (
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/sirupsen/logrus/hooks/test"
"gopkg.in/yaml.v2"
"gorm.io/gorm"
"k8s.io/apimachinery/pkg/types"
)
Expand All @@ -89,7 +92,7 @@ var (
mockOperatorManager *operators.MockAPI
mockHwValidator *hardware.MockValidator
mockIgnitionBuilder *ignition.MockIgnitionBuilder
mockInstallConfigBuilder *installcfg.MockInstallConfigBuilder
mockInstallConfigBuilder *installcfg_builder.MockInstallConfigBuilder
mockStaticNetworkConfig *staticnetworkconfig.MockStaticNetworkConfig
mockProviderRegistry *registry.MockProviderRegistry
secondDayWorkerIgnition = []byte(`{
Expand Down Expand Up @@ -220,8 +223,8 @@ func mockGenerateInstallConfigSuccess(mockGenerator *generator.MockISOInstallCon
}
}

func mockGetInstallConfigSuccess(mockInstallConfigBuilder *installcfg.MockInstallConfigBuilder) {
mockInstallConfigBuilder.EXPECT().GetInstallConfig(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte("some string"), nil).Times(1)
func mockGetInstallConfigSuccess(mockInstallConfigBuilder *installcfg_builder.MockInstallConfigBuilder) {
mockInstallConfigBuilder.EXPECT().GetInstallConfig(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte("{}"), nil).AnyTimes()
}

func addVMToCluster(cluster *common.Cluster, db *gorm.DB) {
Expand Down Expand Up @@ -9385,6 +9388,7 @@ var _ = Describe("UpdateClusterInstallConfig", func() {
eventstest.WithNameMatcher(eventgen.InstallConfigAppliedEventName),
eventstest.WithClusterIdMatcher(params.ClusterID.String())))
mockInstallConfigBuilder.EXPECT().ValidateInstallConfigPatch(gomock.Any(), params.InstallConfigParams).Return(nil).Times(1)
mockGetInstallConfigSuccess(mockInstallConfigBuilder)
mockUsageReports()
response := bm.V2UpdateClusterInstallConfig(ctx, params)
Expect(response).To(BeAssignableToTypeOf(&installer.V2UpdateClusterInstallConfigCreated{}))
Expand Down Expand Up @@ -9431,6 +9435,7 @@ var _ = Describe("UpdateClusterInstallConfig", func() {
eventstest.WithNameMatcher(eventgen.InstallConfigAppliedEventName),
eventstest.WithClusterIdMatcher(params.ClusterID.String())))
mockInstallConfigBuilder.EXPECT().ValidateInstallConfigPatch(gomock.Any(), params.InstallConfigParams).Return(nil).Times(1)
mockGetInstallConfigSuccess(mockInstallConfigBuilder)
bm.V2UpdateClusterInstallConfig(ctx, params)
})

Expand All @@ -9445,12 +9450,118 @@ var _ = Describe("UpdateClusterInstallConfig", func() {
eventstest.WithNameMatcher(eventgen.InstallConfigAppliedEventName),
eventstest.WithClusterIdMatcher(params.ClusterID.String())))
mockInstallConfigBuilder.EXPECT().ValidateInstallConfigPatch(gomock.Any(), params.InstallConfigParams).Return(nil).Times(1)
mockGetInstallConfigSuccess(mockInstallConfigBuilder)
bm.V2UpdateClusterInstallConfig(ctx, params)
var updated common.Cluster
err := db.First(&updated, "id = ?", clusterID).Error
Expect(err).ShouldNot(HaveOccurred())
Expect(updated.Cluster.FeatureUsage).To(Equal(""))
})

DescribeTable(
"Removes the console from the list of monitored operators for 4.12 or newer",
func(version string) {
err := db.Model(&common.Cluster{}).Where("id = ?", clusterID).Update("openshift_version", version).Error
Expect(err).ToNot(HaveOccurred())
operator := &models.MonitoredOperator{
ClusterID: clusterID,
Name: "console",
}
err = db.FirstOrCreate(operator).Error
Expect(err).ToNot(HaveOccurred())
installConfig := installcfg.InstallerConfigBaremetal{
Capabilities: &installcfg.Capabilities{
BaselineCapabilitySet: "None",
AdditionalEnabledCapabilities: []installcfg.ClusterVersionCapability{
"baremetal",
},
},
}
installConfigData, err := yaml.Marshal(installConfig)
Expect(err).ToNot(HaveOccurred())
mockEvents.EXPECT().SendClusterEvent(gomock.Any(), gomock.Any()).AnyTimes()
mockInstallConfigBuilder.EXPECT().ValidateInstallConfigPatch(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
mockInstallConfigBuilder.EXPECT().GetInstallConfig(gomock.Any(), gomock.Any(), gomock.Any()).Return(installConfigData, nil).AnyTimes()
params := installer.V2UpdateClusterInstallConfigParams{
ClusterID: clusterID,
InstallConfigParams: "{}",
}
bm.V2UpdateClusterInstallConfig(ctx, params)
err = db.First(&operator).Error
Expect(err).To(Equal(gorm.ErrRecordNotFound))
},
Entry("Release 4.12", "4.12.7"),
Entry("Release 4.13", "4.13.0"),
Entry("Prerelease 4.13", "4.13.0-ec.5"),
)

DescribeTable(
"Doesn't remove the console from the list of monitored operators for 4.11 or older",
func(version string) {
err := db.Model(&common.Cluster{}).Where("id = ?", clusterID).Update("openshift_version", version).Error
Expect(err).ToNot(HaveOccurred())
operator := &models.MonitoredOperator{
ClusterID: clusterID,
Name: "console",
}
err = db.FirstOrCreate(operator).Error
Expect(err).ToNot(HaveOccurred())
installConfig := installcfg.InstallerConfigBaremetal{
Capabilities: &installcfg.Capabilities{
BaselineCapabilitySet: "None",
AdditionalEnabledCapabilities: []installcfg.ClusterVersionCapability{
"baremetal",
},
},
}
installConfigData, err := yaml.Marshal(installConfig)
Expect(err).ToNot(HaveOccurred())
mockEvents.EXPECT().SendClusterEvent(gomock.Any(), gomock.Any()).AnyTimes()
mockInstallConfigBuilder.EXPECT().ValidateInstallConfigPatch(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
mockInstallConfigBuilder.EXPECT().GetInstallConfig(gomock.Any(), gomock.Any(), gomock.Any()).Return(installConfigData, nil).AnyTimes()
params := installer.V2UpdateClusterInstallConfigParams{
ClusterID: clusterID,
InstallConfigParams: "{}",
}
bm.V2UpdateClusterInstallConfig(ctx, params)
err = db.First(&operator).Error
Expect(err).ToNot(HaveOccurred())
},
Entry("Release 4.10", "4.10.2"),
Entry("Release 4.11", "4.11.3"),
Entry("Prerelease 4.11", "4.11.0-ec.2"),
)

It("Adds the console from the list of monitored operators", func() {
err := db.Model(&common.Cluster{}).Where("id = ?", clusterID).Update("openshift_version", "4.12.7").Error
Expect(err).ToNot(HaveOccurred())
operator := &models.MonitoredOperator{
ClusterID: clusterID,
Name: "console",
}
err = db.Delete(operator).Error
Expect(err).ToNot(HaveOccurred())
installConfig := installcfg.InstallerConfigBaremetal{
Capabilities: &installcfg.Capabilities{
BaselineCapabilitySet: "None",
AdditionalEnabledCapabilities: []installcfg.ClusterVersionCapability{
"Console",
},
},
}
installConfigData, err := yaml.Marshal(installConfig)
Expect(err).ToNot(HaveOccurred())
mockEvents.EXPECT().SendClusterEvent(gomock.Any(), gomock.Any()).AnyTimes()
mockInstallConfigBuilder.EXPECT().ValidateInstallConfigPatch(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
mockInstallConfigBuilder.EXPECT().GetInstallConfig(gomock.Any(), gomock.Any(), gomock.Any()).Return(installConfigData, nil).AnyTimes()
params := installer.V2UpdateClusterInstallConfigParams{
ClusterID: clusterID,
InstallConfigParams: "{}",
}
bm.V2UpdateClusterInstallConfig(ctx, params)
err = db.First(&operator).Error
Expect(err).ToNot(HaveOccurred())
})
})

var _ = Describe("V2DownloadInfraEnvFiles", func() {
Expand Down Expand Up @@ -13435,7 +13546,7 @@ func createInventory(db *gorm.DB, cfg Config) *bareMetalInventory {
mockOperatorManager = operators.NewMockAPI(ctrl)
mockIgnitionBuilder = ignition.NewMockIgnitionBuilder(ctrl)
mockProviderRegistry = registry.NewMockProviderRegistry(ctrl)
mockInstallConfigBuilder = installcfg.NewMockInstallConfigBuilder(ctrl)
mockInstallConfigBuilder = installcfg_builder.NewMockInstallConfigBuilder(ctrl)
mockHwValidator = hardware.NewMockValidator(ctrl)
mockStaticNetworkConfig = staticnetworkconfig.NewMockStaticNetworkConfig(ctrl)
dnsApi := dns.NewDNSHandler(cfg.BaseDNSDomains, common.GetTestLog())
Expand Down