Add --node-selector Flag for shp build/buildrun Commands#309
Conversation
|
cc: @adambkaplan sorry for the delay |
adambkaplan
left a comment
There was a problem hiding this comment.
Great work so far!
I added my review of the items that appear directly related to the node selector change. The bulk of my comments are related to changing the name of the flag to --node-selector. Otherwise this is in very good shape. Once the v1beta1 API PR is merged this can be rebased + unblocked.
|
/ok-to-test |
|
As requested, a new bats test is added for Context: since Doubt: when running successively the following # create mybuild with pre-defined nodeSelector
shp build create mybuild --node-selector="foo=bar" --source-git-url=https://github.com/shipwright-io/sample-go --output-image=my-fake-image
# run mybuild
shp build run mybuildOutcome: apiVersion: shipwright.io/v1beta1
kind: BuildRun
metadata:
creationTimestamp: "2025-04-23T14:40:59Z"
generateName: mybuild-
generation: 1
labels:
build.shipwright.io/generation: "1"
build.shipwright.io/name: mybuild
name: mybuild-4blqb
namespace: default
resourceVersion: "510650"
uid: 2d74cd53-88fd-469c-a44a-1a5d744027f5
spec: # missing .nodeSelector
build:
name: mybuild
status:
buildSpec:
nodeSelector: # present in status
foo: bar
output:
image: my-fake-image
# ...Expectation:
In case of A, we should consider allowing |
4fb92dc to
37984fa
Compare
This is expected behavior. Our API has the following convention for settings that are "shared" across
The |
|
Thank you for the clarification @adambkaplan Everything is sorted on my side. Waiting for #304 to be merged to proceed further. |
37984fa to
c73dfc3
Compare
|
@adambkaplan @SaschaSchwarze0 I have rebased to the latest commit which includes v1beta1. Please trigger the /ok-to-test . Note: this blocks PR 311 The checks are green on my forked repository. |
adambkaplan
left a comment
There was a problem hiding this comment.
/approve
Code changes itself look good! There are only two items IMO that need to be fixed before this merges:
- Revert the changes in the vendor directory (unclear where this newline change came from).
- Squash commits.
vendor/github.com/shipwright-io/build/pkg/apis/build/v1alpha1/build_types.go
Show resolved
Hide resolved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
- test: add bats for `shp build run` - test: add unit tests Signed-off-by: rxinui <rainui.ly@gmail.com>
c73dfc3 to
3027458
Compare
|
@adambkaplan ready to be merged, commit is squashed. |
--node-selector Flag for shp build/buildrun Commands
Changes
Fixes #260
Add CLI flag
--nodeto set.spec.nodeSelectorto aBuildorBuildRun❗ IMPORTANT: depends on PR 304
PR's codebase is tracking PR 304 by @karanibm6 as
.spec.nodeSelectoris only defined onbuilds.shipwright.io/v1beta1as a consequence of the currentmainbranch for CLI relying onbuilds.shipwright.io/v1alpha1(see PR 1683)Submitter Checklist
See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.
Release Notes