Skip to content

MGMT-12471: Don't wait for console if it is disabled #4594

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
Dec 13, 2022
Merged

MGMT-12471: Don't wait for console if it is disabled #4594

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 @@ -37,6 +37,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 @@ -64,6 +65,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 @@ -125,6 +127,7 @@ type Config struct {

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

type Interactivity bool

Expand Down Expand Up @@ -1578,11 +1581,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 @@ -1591,6 +1602,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 @@ -6129,3 +6141,88 @@ func isBaremetalBinaryFromAnotherReleaseImageRequired(cpuArchitecture, version s
featuresupport.IsFeatureSupported(version,
models.FeatureSupportLevelFeaturesItems0FeatureIDARM64ARCHITECTUREWITHCLUSTERMANAGEDNETWORKING)
}

// 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, nil, "")
if err != nil {
return err
}
var installConfig installcfgdata.InstallerConfigBaremetal
Comment thread
eranco74 marked this conversation as resolved.
Outdated
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
Comment thread
jhernand marked this conversation as resolved.
Outdated
} else {
b.log.WithFields(logFields).Info(
"Removing the console from the set of monitored operators",
)
err = tx.Delete(&consoleOperator).Error
Comment thread
jhernand marked this conversation as resolved.
Outdated
}
return err
}
110 changes: 109 additions & 1 deletion internal/bminventory/inventory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ func mockGenerateInstallConfigSuccess(mockGenerator *generator.MockISOInstallCon
}

func mockGetInstallConfigSuccess(mockInstallConfigBuilder *installcfg_builder.MockInstallConfigBuilder) {
mockInstallConfigBuilder.EXPECT().GetInstallConfig(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte("some string"), nil).Times(1)
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 @@ -9869,6 +9869,7 @@ var _ = Describe("UpdateClusterInstallConfig", func() {
eventstest.WithNameMatcher(eventgen.InstallConfigAppliedEventName),
eventstest.WithClusterIdMatcher(params.ClusterID.String())))
mockInstallConfigBuilder.EXPECT().ValidateInstallConfigPatch(gomock.Any(), 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 @@ -9915,6 +9916,7 @@ var _ = Describe("UpdateClusterInstallConfig", func() {
eventstest.WithNameMatcher(eventgen.InstallConfigAppliedEventName),
eventstest.WithClusterIdMatcher(params.ClusterID.String())))
mockInstallConfigBuilder.EXPECT().ValidateInstallConfigPatch(gomock.Any(), gomock.Any(), params.InstallConfigParams).Return(nil).Times(1)
mockGetInstallConfigSuccess(mockInstallConfigBuilder)
bm.V2UpdateClusterInstallConfig(ctx, params)
})

Expand All @@ -9929,12 +9931,118 @@ var _ = Describe("UpdateClusterInstallConfig", func() {
eventstest.WithNameMatcher(eventgen.InstallConfigAppliedEventName),
eventstest.WithClusterIdMatcher(params.ClusterID.String())))
mockInstallConfigBuilder.EXPECT().ValidateInstallConfigPatch(gomock.Any(), 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(), 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(), 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(), 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