Skip to content

use init containers to pull down build inputs #15248

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

Merged
merged 13 commits into from
Aug 16, 2017

Conversation

bparees
Copy link
Contributor

@bparees bparees commented Jul 17, 2017

No description provided.

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 19, 2017
@bparees bparees force-pushed the init_containers3 branch from 35a34d1 to 9f9aadd Compare July 19, 2017 02:13
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 19, 2017
@bparees bparees force-pushed the init_containers3 branch 2 times, most recently from 959d2e4 to 4e9d721 Compare July 21, 2017 20:16
@openshift-merge-robot openshift-merge-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 24, 2017
@bparees bparees force-pushed the init_containers3 branch from dcdecbe to 79c1308 Compare July 24, 2017 19:57
@bparees
Copy link
Contributor Author

bparees commented Jul 24, 2017

[test]

@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bparees

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bparees

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

1 similar comment
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bparees

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@tdawson tdawson removed their assignment Jul 25, 2017
@deads2k
Copy link
Contributor

deads2k commented Jul 26, 2017

/assign bparees
/unassign

@openshift-merge-robot openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 28, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bparees

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 28, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bparees

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 30, 2017
@bparees bparees force-pushed the init_containers3 branch 2 times, most recently from a09aa41 to 8784f17 Compare August 2, 2017 20:04
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2017
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2017
@openshift-merge-robot openshift-merge-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 3, 2017
@bparees
Copy link
Contributor Author

bparees commented Aug 11, 2017

/test extended_builds

@bparees
Copy link
Contributor Author

bparees commented Aug 12, 2017

/test extended_builds

1 similar comment
@bparees
Copy link
Contributor Author

bparees commented Aug 12, 2017

/test extended_builds

@bparees
Copy link
Contributor Author

bparees commented Aug 12, 2017

/test extended_builds

@bparees
Copy link
Contributor Author

bparees commented Aug 14, 2017

/retest

@bparees
Copy link
Contributor Author

bparees commented Aug 14, 2017

/test extended_builds

@bparees
Copy link
Contributor Author

bparees commented Aug 15, 2017

/test extended_builds

1 similar comment
@bparees
Copy link
Contributor Author

bparees commented Aug 15, 2017

/test extended_builds

@bparees
Copy link
Contributor Author

bparees commented Aug 15, 2017

@openshift/devex woo, this is passing all tests including extended builds. please take another look/pass, i'd like to merge it tomorrow so it gets as much soak time in 3.7 as possible.

return sourceSecretDir, nil, fmt.Errorf("cannot fix source secret permissions: %v", errSecret)
}
secretsEnv, overrideURL, err := scmAuths.Setup(sourceSecretDir)
secretsEnv, overrideURL, err := scmAuths.Setup(c.sourceSecretDir)
Copy link
Contributor

Choose a reason for hiding this comment

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

is the TODO comment above still relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, should have been removed, thanks.

Copy link
Contributor

@csrwng csrwng left a comment

Choose a reason for hiding this comment

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

Really minor nit, otherwise lgtm

@@ -185,7 +195,7 @@ func updateBuildRevision(build *buildapi.Build, sourceInfo *git.SourceInfo) *bui

// handleBuildStatusUpdate handles updating the build status
Copy link
Contributor

Choose a reason for hiding this comment

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

s/handle/Handle/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, fixed.

@bparees bparees added the lgtm Indicates that a PR is ready to be merged. label Aug 16, 2017
@bparees
Copy link
Contributor Author

bparees commented Aug 16, 2017

merging on @csrwng's lgtm.

@csrwng
Copy link
Contributor

csrwng commented Aug 16, 2017

don't you need to remove the do-not-merge label?

@bparees
Copy link
Contributor Author

bparees commented Aug 16, 2017

thanks @csrwng :)

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue (batch tested with PRs 15786, 15732, 15248, 15780)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants