Ccip-5112 propose job from ccip jobspec changeset #16184
Conversation
AnieeG
requested review from
winder,
asoliman92,
makramkd,
dimkouv,
mateusz-sekara,
0xAustinWang and
b-gopalswami
AER Report: CI Core ran successfully ✅AER Report: Operator UI CI ran successfully ✅ |
| if err != nil { | ||
| // If we fail to propose a job, we should return an error and the jobs we've already proposed. | ||
| // This is so that we can retry the proposal with manual intervention. | ||
| // JOBID will be empty if the proposal failed. | ||
| return deployment.ChangesetOutput{ | ||
| Proposals: []timelock.MCMSWithTimelockProposal{}, | ||
| AddressBook: nil, | ||
| Jobs: Jobs, | ||
| }, fmt.Errorf("failed to propose job: %w", err) | ||
| } |
There was a problem hiding this comment.
Not a fan of this counter-intuitive logic, returning an error in Go means you should discard return values.
Treating this function as a special one will inevitably lead to mis-usages and bugs
There was a problem hiding this comment.
Fair point. updating this so that on subsequent run, it only proposes jobs which are not previously proposed
There was a problem hiding this comment.
Could not find any option in job distributor to fetch proposals filtered by node. As of now the CCIPJobSpec can be run multiple times without error. There can be two scenarios here -
- Jobs are already accepted by node - in this case CS will not propose it again
- Jobs are not accepted but proposed already - in this CS will propose it again in current implementation until we find a solution to find out if the job is already proposed to the node. It should be alright as same jobs can be proposed multiple times without error and as we are returning job id in cs output, we can only accept the recently proposed jobid returned by the changeset output.
…CCIP-5112-CCIPJobspec
|
Flakeguard SummaryRan new or updated tests between View Flaky Detector Details | Compare Changes Found Flaky Tests ❌
ArtifactsFor detailed logs of the failed tests, please refer to the artifact failed-test-results-with-logs.json. |
Requires
Supports