Skip to content

Updating prow job dryRun variable to be a bool again#238

Merged
rachelvweber merged 1 commit into
mainfrom
rawo/prowjobDryRun
May 21, 2026
Merged

Updating prow job dryRun variable to be a bool again#238
rachelvweber merged 1 commit into
mainfrom
rawo/prowjobDryRun

Conversation

@rachelvweber
Copy link
Copy Markdown
Collaborator

Classic prow tests don't work in fairfax. Ev2 disallows adding execution constraints to validation steps, so I can't exclude them all together. The prow job executor tool already has a dryrun option. It just needs to be plumbed through. This change will require update-testdata to be run in sdp-pipelines but it should be otherwise compatible

Previous history on this in case this looks familiar:

  1. This variable used to be a DryRun-type
  2. The old DryRun variable of type DryRun was removed from the prow job step when the prow job executor was created
  3. The pipelines in ARO-HCP had the DryRun variable removed, but the ones in sdp-pipelines did not
  4. I added the DryRun variable of type bool to this step
  5. Things went poorly when that version of ARO-Tools got to sdp-pipelines because the pipelines still had the old version of the variable
  6. The variable in here was changed back to be of type DryRun to fix the failures
  7. The DryRun variables were removed from the prow job steps in sdp-pipelines
  8. I'm trying this again

Copilot AI review requested due to automatic review settings May 20, 2026 22:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reverts the ProwJob step’s dryRun pipeline field back to a simple boolean to unblock classic prow tests in Fairfax and align with the prow job executor’s --dry-run behavior.

Changes:

  • Changed ProwJobStep.DryRun from a structured DryRun object back to bool.
  • Updated the pipeline JSON schema so ProwJob.dryRun is a boolean.
  • Updated pipeline/test fixtures to use dryRun: true (and removed the old object form).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
pipelines/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml Updates fixtures to reflect ProwJob.dryRun as a boolean and removes invalid {} usages.
pipelines/types/pipeline.schema.v1.json Changes the ProwJob step schema for dryRun from object to boolean.
pipelines/types/common.go Switches ProwJobStep.DryRun to bool and removes DryRun-variable dependency extraction.
pipelines/testdata/pipeline.yaml Updates test pipeline to use dryRun: true for ProwJob steps.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pipelines/types/pipeline.schema.v1.json
Copilot AI review requested due to automatic review settings May 21, 2026 02:28
@rachelvweber rachelvweber reopened this May 21, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@rachelvweber rachelvweber force-pushed the rawo/prowjobDryRun branch 2 times, most recently from b959a26 to 01e4398 Compare May 21, 2026 02:55
Copilot AI review requested due to automatic review settings May 21, 2026 03:08
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

pipelines/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml:584

  • The fixture shows dryRun as an input object with empty required fields (name/resourceGroup/step: "") and includes two e2e2 validation steps. If dryRun is meant to be an input, these empty strings violate the schema’s minLength: 1; if it’s meant to be a bool/value, the fixture shape is wrong. Regenerate the fixture after aligning the dryRun type/schema and ensure each validation step name is unique.
  - action: ProwJob
    dryRun:
      name: ""
      resourceGroup: ""
      step: ""
    gatePromotion: "true"
    identityFrom:
      name: whatever
      resourceGroup: regional
      step: deploy
    jobName: test-me
    name: e2e2
    tokenKeyvault: arohcpint-global.vault.azure.net
    tokenSecret: prow-token
    validation:
    - Internal
  - action: ProwJob
    dryRun:
      name: ""
      resourceGroup: ""
      step: ""
    gatePromotion: "true"
    identityFrom:
      name: whatever
      resourceGroup: regional
      step: deploy
    jobName: test-me
    name: e2e2
    tokenKeyvault: arohcpint-global.vault.azure.net

Comment thread pipelines/types/common.go
Comment thread pipelines/types/pipeline.schema.v1.json Outdated
Comment thread pipelines/testdata/pipeline.yaml
Copy link
Copy Markdown
Contributor

@stevekuznetsov stevekuznetsov left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@rachelvweber rachelvweber merged commit f6ebe44 into main May 21, 2026
2 checks passed
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