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

track base resource pipelines in reconfigure.yml #207

Merged
merged 5 commits into from
Nov 18, 2019

Conversation

jamieklassen
Copy link
Member

contributes to concourse/prod#36

@jamieklassen jamieklassen requested review from a team and vito November 14, 2019 21:14
@zoetian zoetian force-pushed the update-reconfig-piepline branch from cbbc6ed to e0f4a63 Compare November 14, 2019 21:15
RESOURCES:
run:
path: pipelines/tasks/scripts/render-resource-pipeline-templates
user: root
Copy link
Member

@cirocosta cirocosta Nov 15, 2019

Choose a reason for hiding this comment

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

why do we need to explicitly tell that to run as root within the container? is it because bitnami/jsonnet has a different user set by default that concourse doesn't detect?

Copy link
Member Author

Choose a reason for hiding this comment

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

Something wacky about that image, yeah: https://hub.docker.com/r/bitnami/jsonnet/dockerfile the USER 1001 doesn't have permission to read from the task input or write to the task output. Might be worth using a different image, I just liked the sales pitch.

pool
datadog-event
mock
- put : prod
Copy link
Member

@cirocosta cirocosta Nov 15, 2019

Choose a reason for hiding this comment

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

If I got it right, I think we don't need any other inputs other than rendered_pipelines 🤔

Suggested change
- put : prod
- put: prod
inputs: [rendered_pipelines]

not really a big deal though 😁 totally optional

(soon we'll be able to use detect though 👀 concourse/concourse#4616)

Copy link
Member Author

Choose a reason for hiding this comment

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

hahaha I originally had it this way and opted for less code! Maybe it's worth just getting in the habit

cirocosta
cirocosta previously approved these changes Nov 15, 2019
Copy link
Member

@cirocosta cirocosta left a comment

Choose a reason for hiding this comment

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

niiice to see it in a pipeline! LGTM 😁

I just added a minor optional suggestion around explicitly setting inputs to the put step

cirocosta
cirocosta previously approved these changes Nov 15, 2019
Copy link
Member

@vito vito left a comment

Choose a reason for hiding this comment

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

tiny change to make output more clear

tasks/scripts/render-resource-pipeline-templates Outdated Show resolved Hide resolved
Zoe Tian and others added 4 commits November 15, 2019 15:24
Signed-off-by: Zoe Tian <[email protected]>
Co-authored-by: Jamie Klassen <[email protected]>
Signed-off-by: Jamie Klassen <[email protected]>
Signed-off-by: Jamie Klassen <[email protected]>
Co-authored-by: Alex Suraci <[email protected]>
@jamieklassen jamieklassen force-pushed the update-reconfig-piepline branch from 0603875 to 00a6d47 Compare November 15, 2019 20:24
@jamieklassen jamieklassen requested a review from vito November 15, 2019 20:24
@vito vito merged commit 000bc75 into master Nov 18, 2019
@vito vito deleted the update-reconfig-piepline branch November 18, 2019 15:59
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