Skip to content

Commit 803896a

Browse files
Leo6Leoopenshift-cherrypick-robotjhadvig
authored
[release-4.18] OCPBUGS-59721: Disable LightSpeed dialog on non amd64 (#1020)
* [release-4.19] OCPBUGS-58400: Disable LightSpeed dialog on non amd64 architecture (#1011) * feat: implement intelligent capabilities configuration based on cluster architecture * test: add new test for deduplication of node compute environments with multiple architectures * test: update capabilities configuration to configmap test case * test: add unit tests for intelligent capabilities based on node architectures * Apply suggestions from code review Co-authored-by: Jakub Hadvig <[email protected]> * refactor: remove architecture check from capabilities configuration and fix some build issues * refactor: remove redundant admin capabilities check in buildCapabilities function * refactor: enhance capabilities configuration logic and fix the tests * test: remove deprecated capabilities from configmap test case * fmt: run gofmt * refactor: simplify homogeneous architecture check in buildCapabilities function * refactor: improve LightspeedButton capability update logic in buildCapabilities function * test: update TestGetNodeComputeEnvironments tests to use fewer nodes and adjust expected results * fix: fix the review comments * refactor: rethink the logic. using dynamic mechanism to store the supported arch for lightspeed * fix: fix the linter error * refactor: update isLightspeedSupportedArchitecture to check all cluster architectures for support * refactor: refine LightspeedButton capability visibility check in buildCapabilities function * Update pkg/console/subresource/consoleserver/config_builder.go Co-authored-by: Jakub Hadvig <[email protected]> * fix: correct variable scope for architecture support check in isLightspeedSupportedArchitecture function --------- Co-authored-by: Leo6Leo <[email protected]> Co-authored-by: Leo6Leo <[email protected]> Co-authored-by: Jakub Hadvig <[email protected]> * fix: fix the failed unit test introduced by the merging changes --------- Co-authored-by: OpenShift Cherrypick Robot <[email protected]> Co-authored-by: Jakub Hadvig <[email protected]>
1 parent aa56501 commit 803896a

File tree

3 files changed

+179
-12
lines changed

3 files changed

+179
-12
lines changed

pkg/console/operator/sync_v400_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,11 @@ func TestGetNodeComputeEnvironments(t *testing.T) {
201201
t.Run(tt.name, func(t *testing.T) {
202202
actualArchitectures, actualOperatingSystems := getNodeComputeEnvironments(tt.nodeList)
203203
if diff := deep.Equal(tt.expectedArchitectures, actualArchitectures); diff != nil {
204-
t.Error(diff)
205-
return
204+
t.Errorf("Architecture mismatch: %v", diff)
206205
}
207206

208207
if diff := deep.Equal(tt.expectedOperatingSystems, actualOperatingSystems); diff != nil {
209-
t.Error(diff)
210-
return
208+
t.Errorf("OS mismatch: %v", diff)
211209
}
212210
})
213211
}

pkg/console/subresource/consoleserver/config_builder.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ const (
2525
keyFilePath = "/var/serving-cert/tls.key"
2626
)
2727

28+
// SupportedLightspeedArchitectures defines the list of architectures that support Lightspeed.
29+
// Currently only amd64 is supported, but this can be expanded in the future.
30+
var SupportedLightspeedArchitectures = []string{"amd64"}
31+
2832
// ConsoleServerCLIConfigBuilder
2933
// Director will be DefaultConfigMap()
3034
//
@@ -510,7 +514,8 @@ func (b *ConsoleServerCLIConfigBuilder) customization() Customization {
510514
conf.Perspectives = perspectives
511515
}
512516

513-
conf.Capabilities = b.capabilities
517+
// Apply capabilities configuration. This will configure the capability based on the cluster architecture.
518+
conf.Capabilities = b.buildCapabilities()
514519

515520
return conf
516521
}
@@ -549,3 +554,46 @@ func (b *ConsoleServerCLIConfigBuilder) proxy() Proxy {
549554
func (b *ConsoleServerCLIConfigBuilder) Telemetry() map[string]string {
550555
return b.telemetry
551556
}
557+
558+
// buildCapabilities will configure the capabilities based on the cluster architecture.
559+
func (b *ConsoleServerCLIConfigBuilder) buildCapabilities() []operatorv1.Capability {
560+
capabilities := b.capabilities
561+
562+
// Find and configure the LightspeedButton capability
563+
for i := range capabilities {
564+
if capabilities[i].Name == "LightspeedButton" {
565+
if capabilities[i].Visibility.State == operatorv1.CapabilityEnabled && !b.isLightspeedSupportedArchitecture() {
566+
capabilities[i].Visibility.State = operatorv1.CapabilityDisabled
567+
klog.V(4).Infof("disabling LightspeedButton capability - unsupported or mixed architectures: %v", b.nodeArchitectures)
568+
}
569+
break
570+
}
571+
}
572+
573+
return capabilities
574+
}
575+
576+
// isLightspeedSupportedArchitecture checks if all cluster architectures support Lightspeed.
577+
func (b *ConsoleServerCLIConfigBuilder) isLightspeedSupportedArchitecture() bool {
578+
// No architectures means disabled
579+
if len(b.nodeArchitectures) == 0 {
580+
return false
581+
}
582+
583+
// Check if all architectures are supported
584+
isSupported := true
585+
for _, clusterArch := range b.nodeArchitectures {
586+
isSupported = false
587+
for _, supportedArch := range SupportedLightspeedArchitectures {
588+
if clusterArch == supportedArch {
589+
isSupported = true
590+
break
591+
}
592+
}
593+
if !isSupported {
594+
return false
595+
}
596+
}
597+
598+
return isSupported
599+
}

pkg/console/subresource/consoleserver/config_builder_test.go

Lines changed: 128 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,23 @@ import (
1313
corev1 "k8s.io/api/core/v1"
1414
)
1515

16+
// defaultTestCapabilities represents the default capabilities as defined in the operator manifest
17+
// This mirrors the configuration in manifests/01-operator-config.yaml
18+
var defaultTestCapabilities = []v1.Capability{
19+
{
20+
Name: v1.LightspeedButton,
21+
Visibility: v1.CapabilityVisibility{
22+
State: v1.CapabilityEnabled,
23+
},
24+
},
25+
{
26+
Name: v1.GettingStartedBanner,
27+
Visibility: v1.CapabilityVisibility{
28+
State: v1.CapabilityEnabled,
29+
},
30+
},
31+
}
32+
1633
// Tests that the builder will return a correctly structured
1734
// Console Server Config struct when builder.Config() is called
1835
func TestConsoleServerCLIConfigBuilder(t *testing.T) {
@@ -45,11 +62,13 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
4562
Customization: Customization{},
4663
Providers: Providers{},
4764
},
48-
}, {
65+
},
66+
{
4967
name: "Config builder should handle customization with LightspeedButton capability",
5068
input: func() Config {
5169
b := &ConsoleServerCLIConfigBuilder{}
5270
return b.
71+
NodeArchitectures([]string{"amd64"}).
5372
Capabilities([]v1.Capability{
5473
{
5574
Name: v1.LightspeedButton,
@@ -69,7 +88,8 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
6988
KeyFile: keyFilePath,
7089
},
7190
ClusterInfo: ClusterInfo{
72-
ConsoleBasePath: "",
91+
ConsoleBasePath: "",
92+
NodeArchitectures: []string{"amd64"},
7393
},
7494
Auth: Auth{
7595
ClientID: api.OpenShiftConsoleName,
@@ -87,7 +107,8 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
87107
},
88108
Providers: Providers{},
89109
},
90-
}, {
110+
},
111+
{
91112
name: "Config builder should handle cluster info with internal OAuth",
92113
input: func() Config {
93114
b := &ConsoleServerCLIConfigBuilder{}
@@ -121,7 +142,8 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
121142
Customization: Customization{},
122143
Providers: Providers{},
123144
},
124-
}, {
145+
},
146+
{
125147
name: "Config builder should handle cluster info with external OIDC",
126148
input: func() Config {
127149
b := &ConsoleServerCLIConfigBuilder{}
@@ -176,7 +198,8 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
176198
Customization: Customization{},
177199
Providers: Providers{},
178200
},
179-
}, {
201+
},
202+
{
180203
name: "Config builder should handle monitoring and info",
181204
input: func() Config {
182205
b := &ConsoleServerCLIConfigBuilder{}
@@ -222,7 +245,8 @@ func TestConsoleServerCLIConfigBuilder(t *testing.T) {
222245
Customization: Customization{},
223246
Providers: Providers{},
224247
},
225-
}, {
248+
},
249+
{
226250
name: "Config builder should handle StatuspageID",
227251
input: func() Config {
228252
b := &ConsoleServerCLIConfigBuilder{}
@@ -1508,7 +1532,7 @@ customization:
15081532
capabilities:
15091533
- name: LightspeedButton
15101534
visibility:
1511-
state: Enabled
1535+
state: Disabled
15121536
providers: {}
15131537
`,
15141538
},
@@ -1517,8 +1541,105 @@ providers: {}
15171541
t.Run(tt.name, func(t *testing.T) {
15181542
input, _ := tt.input()
15191543
if diff := cmp.Diff(tt.output, string(input)); len(diff) > 0 {
1544+
15201545
t.Error(diff)
15211546
}
15221547
})
15231548
}
15241549
}
1550+
1551+
func TestBuildCapabilities(t *testing.T) {
1552+
tests := []struct {
1553+
name string
1554+
nodeArchitectures []string
1555+
expectedCapabilities []v1.Capability
1556+
}{
1557+
{
1558+
name: "Homogeneous AMD64 cluster - enables Lightspeed",
1559+
nodeArchitectures: []string{"amd64"},
1560+
expectedCapabilities: []v1.Capability{
1561+
{
1562+
Name: "LightspeedButton",
1563+
Visibility: v1.CapabilityVisibility{
1564+
State: v1.CapabilityEnabled,
1565+
},
1566+
},
1567+
{
1568+
Name: "GettingStartedBanner",
1569+
Visibility: v1.CapabilityVisibility{
1570+
State: v1.CapabilityEnabled,
1571+
},
1572+
},
1573+
},
1574+
},
1575+
{
1576+
name: "Mixed architecture cluster - disables Lightspeed",
1577+
nodeArchitectures: []string{"amd64", "ppc64le"},
1578+
expectedCapabilities: []v1.Capability{
1579+
{
1580+
Name: "LightspeedButton",
1581+
Visibility: v1.CapabilityVisibility{
1582+
State: v1.CapabilityDisabled,
1583+
},
1584+
},
1585+
{
1586+
Name: "GettingStartedBanner",
1587+
Visibility: v1.CapabilityVisibility{
1588+
State: v1.CapabilityEnabled,
1589+
},
1590+
},
1591+
},
1592+
},
1593+
{
1594+
name: "Power-only cluster - disables Lightspeed",
1595+
nodeArchitectures: []string{"ppc64le"},
1596+
expectedCapabilities: []v1.Capability{
1597+
{
1598+
Name: "LightspeedButton",
1599+
Visibility: v1.CapabilityVisibility{
1600+
State: v1.CapabilityDisabled,
1601+
},
1602+
},
1603+
{
1604+
Name: "GettingStartedBanner",
1605+
Visibility: v1.CapabilityVisibility{
1606+
State: v1.CapabilityEnabled,
1607+
},
1608+
},
1609+
},
1610+
},
1611+
{
1612+
name: "Empty node architectures - disables Lightspeed",
1613+
nodeArchitectures: []string{},
1614+
expectedCapabilities: []v1.Capability{
1615+
{
1616+
Name: "LightspeedButton",
1617+
Visibility: v1.CapabilityVisibility{
1618+
State: v1.CapabilityDisabled,
1619+
},
1620+
},
1621+
{
1622+
Name: "GettingStartedBanner",
1623+
Visibility: v1.CapabilityVisibility{
1624+
State: v1.CapabilityEnabled,
1625+
},
1626+
},
1627+
},
1628+
},
1629+
}
1630+
1631+
for _, tt := range tests {
1632+
t.Run(tt.name, func(t *testing.T) {
1633+
builder := &ConsoleServerCLIConfigBuilder{
1634+
nodeArchitectures: tt.nodeArchitectures,
1635+
capabilities: defaultTestCapabilities,
1636+
}
1637+
1638+
result := builder.buildCapabilities()
1639+
1640+
if diff := deep.Equal(tt.expectedCapabilities, result); diff != nil {
1641+
t.Errorf("Expected capabilities don't match: %v", diff)
1642+
}
1643+
})
1644+
}
1645+
}

0 commit comments

Comments
 (0)