Skip to content

Fix incorrect default OS image selection when Ubuntu is disabled#7927

Open
KhizerRehan wants to merge 2 commits intokubermatic:mainfrom
KhizerRehan:fix/node-data-remove-hardcoded-ubuntu-default
Open

Fix incorrect default OS image selection when Ubuntu is disabled#7927
KhizerRehan wants to merge 2 commits intokubermatic:mainfrom
KhizerRehan:fix/node-data-remove-hardcoded-ubuntu-default

Conversation

@KhizerRehan
Copy link
Contributor

@KhizerRehan KhizerRehan commented Mar 12, 2026

What this PR does / why we need it:
This PR fixes incorrect OS defaulting behavior where Nutanix and OpenStack node data components always reset the operating system to Ubuntu and checking/selecting first next allowed operating system

Pre-requisits:

  • Disable ubuntu OS from Admin Panel (Globally)

Try to create a cluster with Ubuntu disabled

Before:

Screenshot 2026-03-12 at 2 25 18 PM
Screenshot 2026-03-12 at 2 25 40 PM

After

Screenshot 2026-03-12 at 2 25 51 PM

Which issue(s) this PR fixes:

Fixes #7242

Testing

  • Admin Disables Ubuntu e.g (OpenStack)
  • Log in as an admin.
  • Navigate to Admin Settings → Allowed Operating Systems.
  • Uncheck Ubuntu (disable it globally).
  • Create a new cluster using the OpenStack provider.
  • Navigate to Initial Nodes step.

Expected: First allowed OS is auto-selected; the image field shows the correct image for that OS (not Ubuntu) and shows correct OSP selected based on OS.

What type of PR is this?
/kind bug

Special notes for your reviewer:

Does this PR introduce a user-facing change? Then add your Release Note here:

Fixed the default OS image selection to correctly use the enabled OS when Ubuntu is disabled globally or per project.

Documentation:

NONE

@kubermatic-bot kubermatic-bot added kind/bug Categorizes issue or PR as related to a bug. release-note Denotes a PR that will be considered when it comes time to generate release notes. docs/none Denotes a PR that doesn't need documentation (changes). dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. labels Mar 12, 2026
@kubermatic-bot
Copy link
Contributor

kubermatic-bot commented Mar 12, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ahmedwaleedmalik for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubermatic-bot kubermatic-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 12, 2026
@KhizerRehan KhizerRehan force-pushed the fix/node-data-remove-hardcoded-ubuntu-default branch from a9144a8 to b32d378 Compare March 12, 2026 10:02
@KhizerRehan KhizerRehan added this to the KKP 2.31 milestone Mar 12, 2026
KhizerRehan
KhizerRehan commented Mar 12, 2026
View reviewed changes
modules/web/src/app/node-data/basic/provider/vsphere/component.ts
}
this.initiallySelectedOS = null;

switch (os) {
Copy link
Contributor Author

@KhizerRehan KhizerRehan Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored in favour of single method since we are getting default template from DatacenterOperatingSystemOptions which shares same type as of Images as above provider options.

let me know if any concerns.

@KhizerRehan
Copy link
Contributor Author

KhizerRehan commented Mar 12, 2026

/test pre-dashboard-web-integration-tests-ce

ahmadhamzh
ahmadhamzh reviewed Mar 13, 2026
View reviewed changes
modules/web/src/app/node-data/component.ts Outdated

if (this.wizardMode && this.wizardMode !== WizardMode.CreateClusterTemplate) {
const currentOS = this._nodeDataService.operatingSystem;
if (currentOS && this.isOperatingSystemSupported(currentOS) && this.allowedOperatingSystems[currentOS]) {
Copy link
Contributor

@ahmadhamzh ahmadhamzh Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check the method isOperatingSystemAllowed we do show the current os system even if it's not allowed
this.currentNodeOS === os
if we want the OS to be shown in the edits/customize cluster template modes then shouldn't we keep it as selected by default?

Copy link
Contributor Author

@KhizerRehan KhizerRehan Mar 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to clarify an edge case

  • User selects e.g Ubuntu as OS while creating a template
  • Admin or At Project level that OS is disabled
  • User tries to edit -> same template which has Ubuntu selected

What should be the behaviour in case it is disabled

  • At Admin or Project level

Shouldn't we explicilty remove OS even from edit/customize of template becasue OS was not allowed from (Project or Admin Settings)

any thoughts on this?

cc: @ahmadhamzh: I have simplified logic let me know wdyt?

Copy link
Contributor

@ahmadhamzh ahmadhamzh Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be the same behavior as before, so the selected option should be shown because I think that is not related to the bug ticket.

modules/web/src/app/shared/utils/provider.ts
@@ -0,0 +1,35 @@
// Copyright 2026 The Kubermatic Kubernetes Platform contributors.
Copy link
Contributor

@ahmadhamzh ahmadhamzh Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not sure about this file name what it should contain why you consider the getDefaultForOS is something regarding provider, shouldn't we name the file e.g node-data , or maybe we can add the function in this file src/app/shared/utils/node.ts WDYT ?

Copy link
Contributor Author

@KhizerRehan KhizerRehan Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

KhizerRehan
KhizerRehan commented Mar 13, 2026
View reviewed changes
Copy link
Contributor Author

@KhizerRehan KhizerRehan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback Response(PTAL)

modules/web/src/app/shared/utils/provider.ts
@@ -0,0 +1,35 @@
// Copyright 2026 The Kubermatic Kubernetes Platform contributors.
Copy link
Contributor Author

@KhizerRehan KhizerRehan Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

modules/web/src/app/node-data/component.ts Outdated

if (this.wizardMode && this.wizardMode !== WizardMode.CreateClusterTemplate) {
const currentOS = this._nodeDataService.operatingSystem;
if (currentOS && this.isOperatingSystemSupported(currentOS) && this.allowedOperatingSystems[currentOS]) {
Copy link
Contributor Author

@KhizerRehan KhizerRehan Mar 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to clarify an edge case

  • User selects e.g Ubuntu as OS while creating a template
  • Admin or At Project level that OS is disabled
  • User tries to edit -> same template which has Ubuntu selected

What should be the behaviour in case it is disabled

  • At Admin or Project level

Shouldn't we explicilty remove OS even from edit/customize of template becasue OS was not allowed from (Project or Admin Settings)

any thoughts on this?

cc: @ahmadhamzh: I have simplified logic let me know wdyt?

@KhizerRehan KhizerRehan force-pushed the fix/node-data-remove-hardcoded-ubuntu-default branch from d950bd2 to d214ec8 Compare March 13, 2026 14:43
@kubermatic-bot kubermatic-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 13, 2026
@kubermatic-bot kubermatic-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 13, 2026
@KhizerRehan
Copy link
Contributor Author

KhizerRehan commented Mar 16, 2026

/test pre-dashboard-web-integration-tests-ce

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahmadhamzh ahmadhamzh ahmadhamzh left review comments

Assignees

No one assigned

Labels

dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. docs/none Denotes a PR that doesn't need documentation (changes). kind/bug Categorizes issue or PR as related to a bug. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

KKP 2.31

Development

Successfully merging this pull request may close these issues.

Wrong default image name used if Ubuntu is disabled

3 participants

@KhizerRehan @kubermatic-bot @ahmadhamzh