Skip to content

Conversation

briankane
Copy link
Contributor

@briankane briankane commented Jun 11, 2025

… logic

Description of your changes

  1. Fixes issues with addon-operation not waiting for correct status reporting of the created Job (always showing as failed despite the addon succeeding).
  2. Updates the image in use (latest version) as step fails with the current image.

Fixes #

  1. Addon-Operation - WFR Status Always Shows Failed #207
  2. Addon-Operation Default Image - Pods Fail #208

I have:

  • Read and followed KubeVela's contribution process.
  • [] ~~Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary. ~~
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

  • Reproduced issues locally with existing workflow step.
  • Deployed updated workstep step.
  • Confirmed addons are now created as expected and the WorkflowRun & Step correctly reflect the successful installation of the addon.

Special notes for your reviewer

N/A


Summary by cubic

Updated the addon-operation workflow step to use the latest image and fixed the wait logic so job status is reported correctly.

  • Bug Fixes

    • Fixed issue where the step always showed as failed even when the addon succeeded.
    • Improved wait logic to check both succeeded and failed job statuses.
  • Dependencies

    • Updated the default image to oamdev/vela-cli:v1.10.3.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

cubic reviewed this PR and found no issues. Review PR in cubic.dev.

@briankane briankane force-pushed the fix/addon-operation-workflowstep-fixes branch 2 times, most recently from 9dae36f to 7029070 Compare June 11, 2025 07:33
@briankane briankane force-pushed the fix/addon-operation-workflowstep-fixes branch from 7029070 to 56e0ac6 Compare July 4, 2025 15:15
Copy link

codecov bot commented Jul 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 20.32%. Comparing base (d7db9c4) to head (2b844bb).
Report is 2 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (d7db9c4) and HEAD (2b844bb). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (d7db9c4) HEAD (2b844bb)
unit-test 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #210       +/-   ##
===========================================
- Coverage   62.49%   20.32%   -42.17%     
===========================================
  Files          62       59        -3     
  Lines        4415     5637     +1222     
===========================================
- Hits         2759     1146     -1613     
- Misses       1324     4316     +2992     
+ Partials      332      175      -157     
Flag Coverage Δ
e2etests 20.32% <ø> (?)
unit-test ?

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

☔ View full report in Codecov by Sentry.
📢 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.

@briankane briankane force-pushed the fix/addon-operation-workflowstep-fixes branch from 56e0ac6 to 2b844bb Compare July 9, 2025 10:56
@briankane briankane requested a review from FogDong July 9, 2025 22:52
wait: op.#ConditionalWait & {
continue: job.value.status.succeeded != _|_ && job.value.status.succeeded > 0
continue: job.value.status.succeeded != _|_ || job.value.status.failed != _|_
Copy link

Choose a reason for hiding this comment

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

In a workflow with several steps, turning on continue on fail lets the next steps start even if the current step hasn’t finished or succeeded yet. This way, the workflow doesn’t wait for all retry attempts to be used up before moving on.

Copy link

Choose a reason for hiding this comment

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

I ended up using the new providers with if statements to check the status.succeeded is set. This snippet working for me.

import (
	"strings"
	"vela/builtin"
	"vela/kube"
	"vela/util"
)
....
       job: op.#Apply & {
                $params: value: {
        ....
        }
	log: util.#Log & {
		$params: source: {
			resources: [{labelSelector: {
				"workflow.oam.dev/name":    context.name
				"workflow.oam.dev/session": context.stepSessionID
			}}]
		}
	}

	fail: builtin.#Steps & {
		if job.$returns.value.status.failed != _|_ {
			if job.$returns.value.status.failed > 2 {
				breakWorkflow: builtin.#Fail & {
					$params: message: "addon operation failed"
				}
			}
		}

	wait: builtin.#ConditionalWait & {
		if job.$returns.value.status.succeeded != _|_ {
			if job.$returns.value.status.succeeded > 0 {
				$params: continue: true
			}
		}
	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants