Skip to content
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.

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

Fix failure when reporting final action: #632

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

jacobweinstock
Copy link
Member

@jacobweinstock jacobweinstock commented Jul 1, 2022

Description

This allows the final action in a workflow to successfully report its status and consequently allow a workflow to complete.
This issue: #559 incorrectly describes a fix that was applied in this PR: #576

Why is this needed

Fixes: tinkerbell/playground#143

How Has This Been Tested?

Unit tests were added and the sandbox (vagrant virtualbox quickstart) was used to manually test.

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

@jacobweinstock jacobweinstock added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. kind/regression Categorizes issue or PR as related to a regression from a prior release. labels Jul 1, 2022
@codecov
Copy link

codecov bot commented Jul 1, 2022

Codecov Report

Merging #632 (632e166) into main (fe1eba2) will decrease coverage by 0.19%.
The diff coverage is 100.00%.

❗ Current head 632e166 differs from pull request most recent head 8c5a987. Consider uploading reports for the commit 8c5a987 to get more accurate results

@@            Coverage Diff             @@
##             main     #632      +/-   ##
==========================================
- Coverage   44.63%   44.43%   -0.20%     
==========================================
  Files          61       61              
  Lines        3524     3497      -27     
==========================================
- Hits         1573     1554      -19     
+ Misses       1869     1862       -7     
+ Partials       82       81       -1     
Impacted Files Coverage Δ
server/dbserver_worker_workflow.go 91.36% <100.00%> (+1.43%) ⬆️
pkg/apis/core/v1alpha1/workflow_methods.go 75.00% <0.00%> (-1.28%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe1eba2...8c5a987. Read the comment docs.

This allows the final action in a workflow to successfully
report its status and consequently allow a workflow to complete.

Signed-off-by: Jacob Weinstock <[email protected]>
Signed-off-by: Manuel Mendez <[email protected]>
@mmlb mmlb force-pushed the final-action-report-error branch from f76203d to 8c5a987 Compare July 6, 2022 13:40
@mmlb
Copy link
Contributor

mmlb commented Jul 6, 2022

Thanks, I changed the > ... foo-1 to just >= foo for legibility. No changes to the test you added and still passes.

@mmlb mmlb added the ready-to-merge Signal to Mergify to merge the PR. label Jul 6, 2022
@mergify mergify bot merged commit 1618650 into tinkerbell:main Jul 6, 2022
mergify bot added a commit to tinkerbell/playground that referenced this pull request Aug 2, 2022
## Description
The current image that is used in the main branch doesn't work due to the issue which is reported here: #143. There was a PR which addressed this issue already which can be found here tinkerbell/tink#632. This PR uses the latest `tink-server`, `tink-worker` and `tink-cli` which has the fix already in place. 


## Why is this needed



Fixes: #

## How Has This Been Tested?



- Checked out the main branch  
- Update the env var `vTINK` to `sha-16186501` 
- Run docker compose 
- Provision all the needed manifests(HW, WFL, TPL) 
- Reboot the machine 

## How are existing users impacted? What migration steps/scripts do we need?
I haven't updated any other components or image except the tink image.




## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade
@displague displague added this to the 0.7.0 milestone Aug 15, 2022
ttwd80 pushed a commit to ttwd80/tinkerbell-playground that referenced this pull request Sep 7, 2024
…ell#146)

## Description
The current image that is used in the main branch doesn't work due to the issue which is reported here: tinkerbell#143. There was a PR which addressed this issue already which can be found here tinkerbell/tink#632. This PR uses the latest `tink-server`, `tink-worker` and `tink-cli` which has the fix already in place. 


## Why is this needed



Fixes: #

## How Has This Been Tested?



- Checked out the main branch  
- Update the env var `vTINK` to `sha-16186501` 
- Run docker compose 
- Provision all the needed manifests(HW, WFL, TPL) 
- Reboot the machine 

## How are existing users impacted? What migration steps/scripts do we need?
I haven't updated any other components or image except the tink image.




## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/regression Categorizes issue or PR as related to a regression from a prior release. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. ready-to-merge Signal to Mergify to merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quick start with EM stuck at 87%; can't complete write-netplan step
3 participants