Skip to content

CKS: don't fail when a node still has an ISO attached - #14637

Open
bhouse-nexthop wants to merge 1 commit into
apache:mainfrom
bhouse-nexthop:cks-iso-already-attached
Open

bhouse-nexthop wants to merge 1 commit into
apache:mainfrom
bhouse-nexthop:cks-iso-already-attached

Conversation

@bhouse-nexthop

Copy link
Copy Markdown
Collaborator

Description

Since #13101 (multiple CD-ROMs per VM), attachIso on main refuses an ISO that is already attached, and any ISO beyond the number of CD-ROMs, one by default. KubernetesClusterActionWorker.attachIsoKubernetesVMs relied on the attach replacing whatever ISO a node had. So a node left holding the binaries ISO by an operation that didn't finish, for example because the management server stopped, made every later upgrade, scale-up or start fail before it started, with "The specified ISO is already attached to this Instance."

Fixes #14630

For each node, attachIsoKubernetesVMs now reads the VM again and:

  • leaves the binaries ISO in place if it is already attached, in the primary CD-ROM or in another one;
  • otherwise detaches the ISO in the primary CD-ROM, if any, before attaching the binaries ISO, as the attach used to replace it.

4.22 is not affected, so this is for main only.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

How Has This Been Tested?

New unit tests in KubernetesClusterActionWorkerIsoTest:

  • a node with no ISO gets the binaries ISO attached, and nothing is detached;
  • a node that already has the binaries ISO, in the primary CD-ROM or in another one, is left as it is;
  • a node with another ISO has it detached, then the binaries ISO attached.

Without the fix, the last three fail. mvn -pl plugins/integrations/kubernetes-service -am test -Dtest='KubernetesClusterActionWorker*Test' passes.

On a test environment running main, an upgrade failed as described in #14630 while both nodes still held the binaries ISO, and ran once the ISO was detached from both by hand. This PR itself has not been run against a live cluster.

How did you try to break this feature and the system with this change?

  • A node with no ISO takes the same path as before.
  • If the detach fails, the operation fails as an attach failure did before.
  • If the attach fails after the detach, the node is left with no ISO, and the operation fails as before.
  • detachIsoKubernetesVMs is unchanged.

attachIsoKubernetesVMs attaches the binaries ISO to every node before
an upgrade, a scale-up or a start, and relied on the attach replacing
whatever ISO the node had. Since an Instance can have several CD-ROMs,
attachIso refuses an ISO that is already attached, and any ISO beyond
the number of CD-ROMs, one by default. A node left holding the binaries
ISO by an operation that did not finish, for example because the
management server stopped, then made every later operation fail before
it started.

Leave the binaries ISO in place if the node has it already, and detach
any other ISO from the node first, as the attach used to replace it.

Signed-off-by: Brad House <[email protected]>
@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 20.78%. Comparing base (a95eb59) to head (e9f5d4e).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...r/actionworkers/KubernetesClusterActionWorker.java 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main   #14637   +/-   ##
=========================================
  Coverage     20.78%   20.78%           
- Complexity    21927    21932    +5     
=========================================
  Files          6496     6496           
  Lines        591657   591665    +8     
  Branches      72592    72595    +3     
=========================================
+ Hits         122955   122967   +12     
+ Misses       455356   455345   -11     
- Partials      13346    13353    +7     
Flag Coverage Δ
uitests 3.88% <ø> (ø)
unittests 22.09% <87.50%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CKS (main): an upgrade, scale or start fails if a node still has an ISO attached

1 participant