-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Breakup builds.adoc into subdir #3739
Conversation
_topic_map.yml
Outdated
File: build_strategies | ||
- Name: Triggering Builds | ||
File: triggering_builds | ||
- Name: Secrets During Builds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I suggest something like "Using Secrets with Builds"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah "Secrets During Builds" is weird. "Using Secrets with Builds" is what it was originally, was trying to come up with something different... Placeholder for now. I bet it'll just end up back at "Using Secrets During Builds" ultimately.
_topic_map.yml
Outdated
File: how_builds_work | ||
- Name: Basic Build Operations | ||
File: basic_build_operations | ||
- Name: Build Inputs and Output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not sure what this section is about... Maybe something like "Build Plug-ins" or "Using Builds with Outside Sources" or something. It'd help if I fully understood what the topic was about. Sorry :|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's about the type of inputs that you can feed into a build and the output that result from it.
@adellape I agree with this all. I made some suggestions for the titles of the topics. I know it's largely copied over from the existing stuff, but it'd be a good chance to make them a bit more descriptive. |
076d083
to
c9972a6
Compare
Updated preview: http://file.rdu.redhat.com/~adellape/021417/buildsfinally/dev_guide/builds/ @bparees Can you take a look at how this is shaping up? |
c9972a6
to
3972db7
Compare
setting a maximum duration seems closely related to setting build resources, so i'd consider moving the build resources section out of "how builds work" and into "basic build operations". Both topics could also be moved into the "advanced usage" section instead. on inputs/outputs i'd like to see the proxy+secrets subsections linked in the index at the top build-secrets are really just another build input, so that content should probably be refactored from here: of course that leaves just the docker push/pull secrets content in the "build secrets" topic which leaves me thinking that topic needs to be refactored. there's really 3 separate discussions of secrets in the build docs:
You've put (1) under build inputs and 2+3 under "build secrets" but 2+3 aren't really related to each other any more than they are to (1). (there's actually a 4th discussion of secrets too, in the custom strategy options section. That use of secrets is most closely related to (3), but the mechanism is slightly different) |
3972db7
to
1d5bf64
Compare
@bparees Thanks for looking. Inline:
Both moved to "Advanced Build Operations".
I've now set
Would it be fine to move all of what's currently in the "Build Secrets" topic into "Inputs and Output" (therefore 1, 2, and 3 in your list are all in that topic at that point)? Maybe pull the "* Source" sections up a section-level... The reason I put "Inputs and Output" together was because it didn't seem like there was enough for output to warrant its own topic, but at this point it's seeming like there will be a lot of content in here for inputs, so probably worth splitting them apart after all. |
yup that's what i meant.
i think it's fine as is for now, but splitting inputs and outputs into separate top level pages is certainly something to consider in the future if things continue to grow.
yes. Incidentally while skimming that Build Secrets section I see we have a terminology problem. We use "source secrets" elsewhere to mean "a secret used to access your source(git repo)". Then in the Build Secrets section we use "source secrets" to mean "a secret that is provided as a source input". We should disambiguate that. When referring to a secret that's being used a source input, maybe we should call it an input secret or something. Not sure. It's extra difficult since the first one is specified via spec.source.sourceSecrets(used to access git) and the second one is in spec.source.secrets(provided to the build to be used for whatever the build wants to do with it)
i'm ok either way. |
`BuildConfig` is typically generated automatically for you if you use the web | ||
console or CLI, and it can be edited at any time. Understanding the parts that | ||
make up a `BuildConfig` and their available options can help if you choose to | ||
manually tweak your configuration later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bparees I added this 2nd paragraph here and renamed the section from "Defining a BuildConfig" -> "What Is a BuildConfig?" because it initially seemed potentially like we were expecting you to go craft a BuildConfig from scratch before doing anything w/ builds. Hopefully this makes it a little clearer what the expectations are. Any suggestions on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works for me.
dev_guide/builds/index.adoc
Outdated
xref:build_inputs_output.adoc#binary-source[how it is specified to the system]. | ||
|
||
[[defining-a-buildconfig]] | ||
== What Is a BuildConfig? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lowercase i or uppercase a?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how title case says to do it, apparently. 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ick. but ok :)
da35102
to
ebfb0aa
Compare
endif::[] | ||
|
||
[[source-clone-secrets]] | ||
=== Source Clone Secrets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bparees I noticed in dev_guide/secrets.adoc
we referred to these as "source clone secrets". That alright? Or is the clone part of it too narrow of a name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other maybes: Git Source Secrets, Git Repo Secrets...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clone is reasonable, that's really what we're using it for...the git clone operation. it's just a bit confusing since the api doesn't mention "clone".
dev_guide/builds/build_inputs.adoc
Outdated
|
||
|
||
[[using-secrets-during-build]] | ||
== Input Secrets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bparees Went with "Input Secrets" here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k
dev_guide/builds/build_inputs.adoc
Outdated
[[using-secrets-s2i-strategy]] | ||
=== Source-to-Image Strategy | ||
|
||
When using a `Source` strategy, all defined source secrets are copied to their |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bparees In this and the next 2 subsections, the instances of "source secrets" should become "input secrets", yes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
ebfb0aa
to
68cf83f
Compare
input secrets needs to be mentioned in the bullet list in the overview of the build inputs topic (and i guess that whole paragraph should be reworked to include what happens w/ input secrets). on the output side, http://file.rdu.redhat.com/%7Eadellape/021417/buildsfinally/dev_guide/builds/build_output.html#output-image-environment-variables should also make mention of the fact that any user defined env vars from things like http://file.rdu.redhat.com/%7Eadellape/021417/buildsfinally/dev_guide/builds/build_strategies.html#configuring-the-source-environment and http://file.rdu.redhat.com/%7Eadellape/021417/buildsfinally/dev_guide/builds/build_strategies.html#docker-strategy-environment will also be part of the output image environment variable list. (the strategy sections do already note this, saying " The environment variables defined there are visible during the assemble script execution and will be defined in the output image, making them also available to the run script and application code." and "The variables are defined during build and stay in the output image, therefore they will be present in any container that runs that image as well.") |
68cf83f
to
b479e17
Compare
@bfallonf @ahardin-rh @bmcelvee @aheslin @mburke5678 I think I've touched enough of this by now to call it a rough first draft, feel free to peer review. Be warned I haven't done a comprehensive link check yet (I've updated all links inside these files but haven't gone through the rest of the repo to fix everything that links back to the old Preview build (may need to refresh if you've linked here previously): http://file.rdu.redhat.com/~adellape/021417/buildsfinally/dev_guide/builds/ |
this is great stuff @adellape |
generic: | ||
secret: "secret101" | ||
- | ||
type: "ImageChange" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "type" stuff is a little screwy here, format-wise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this YAML formatting come up before with @bparees... IIRC, this is how stuff gets reformatted in OpenShift. Or at least, OpenShift will accept this form. Think I'll leave any potential changes along these lines to another PR.
Much better than the original @adellape ! Will take a closer look. |
---- | ||
$ oc describe quota | ||
---- | ||
|=== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is like this currently in the docs, but would it be better to take this out of a table and into an actual section? I think a table with just one thing is a little redundant, plus, having it like this isn't really consistent with the rest of the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it does look fairly weird today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, though I couldn't remember if someone had already taken it from section -> table, and I'd be putting it back into sections again. But I do think the table is weird, I'll change it.
0fb8cec
to
114f936
Compare
toc::[] | ||
|
||
[[how-build-inputs-work]] | ||
== How Build Inputs Work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-titled "Providing Source for a Build" -> "How Build Inputs Work", since "Providing" made it seem like there was actually a task in here, which there is not.
dev_guide/builds/build_inputs.adoc
Outdated
Input secrets are useful for when you do not want certain resources or | ||
credentials used during a build to be available in the final application image | ||
produced by the build. External artifacts outside of a source repository can | ||
also be used to pull in additional files during a build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bparees I added "Input secrets" and "External artifacts" to the above list. Some questions:
-
Is it problematic to try adding these 2 to the list since it's supposed to be "In order of precedence"? Where would they fall, or is that not really helpful?
-
I reordered the topic's later section order to match the precedence list above. I assume we had Git Source first previously because it's the most common. Any issues w/ reordering?
-
I added this above paragraph to touch briefly on input secrets and external artifacts since they were new additions and the other 4 had gotten touched on in the preceding paragraph. Any issues w/ the summary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(1) slightly. using precedence was already problematic though... (eg git source and binary source are mutually exclusive, so there's no applicable "precedence" there).
(2) it's not just the most common but also the most universally applicable (having an inline dockerfile w/ an s2i build doesn't usually make sense. having a git repo always makes sense), but i think keeping the ordering symmetric is sensible.
(3) slight rewordings/augmentation:
"Input secrets are useful for when you do not want certain resources or
credentials used during a build to be available in the final application image
produced by the build or want to consume a value that is defined in a Secret resource. External artifacts can be used to pull in additional files that are not available as one of the other input types."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
. The content from the current directory is provided to the build process | ||
for reference by the Dockerfile, *_assemble_* script, or custom builder logic. | ||
This means any input content that resides outside the `contextDir` will be | ||
ignored by the build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bparees Do we need to try adding in mention of build inputs and external artifacts into this numbered list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no. external artifacts wouldn't be relevant here and i think "input content" covers the rest sufficiently.
`$BUILD` environment variable, which includes the full build object. | ||
|
||
[[using-external-artifacts]] | ||
== Using External Artifacts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropped the "During a Build" part of this section title since it was redundant in context.
|
||
|
||
[[using-docker-credentials-for-private-registries]] | ||
== Using Docker Credentials for Private Registries |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bparees Any issues with this re-title? It was previously "Using Docker Credentials for Pushing and Pulling Images", which seemed overlong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plus, given the [NOTE]
we have about how it's not required for internal registry use, it seemed to make sense to clarify in the section title that this is for private registry use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks fine.
- xref:build_inputs.adoc#binary-source[Binary] | ||
- xref:build_inputs.adoc#image-source[Image] | ||
- xref:build_inputs.adoc#using-secrets-during-build[Input secrets] | ||
- xref:build_inputs.adoc#using-external-artifacts[External artifcats] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added input secrets and external artifacts here.
754ad24
to
9249ed6
Compare
@bparees I'm trying to land these general changes for Origin + 3.4, but also making sure that merged 3.5-content doesn't get into 3.4 too early. I went back through the Using URLs with start-build --from-file/--from-dir (#3184) I've removed that related content from this PR, and will make a follow-up PR after this one merges containing the 3.5+ stuff, merge that to master, then label that new PR for inclusion in OCP starting with 3.5 GA. 😵 So my question: #3184 was easy enough to cut out. Can you confirm that what I've cut out for the #3465 changes is fine? That's currently in this separate commit: 9249ed6. Basically I removed the "Automatically Adding Source Clone Secrets" subsection, and left the "Manually Adding Source Clone Secrets" as-is, which still seemed accurate AFAICT. |
@adellape yup that looks right. |
9249ed6
to
7bfe83b
Compare
7bfe83b
to
a024e92
Compare
a024e92
to
79cae27
Compare
@bmcelvee @ahardin-rh @mburke5678 @bfallonf @vikram-redhat @aheslin @bparees Please be aware that this has merged to master, so any new work going forward related to "Builds" should now go into a This will show up in the Origin docs after the nightly auto-builds later tonight, and I'll take care of cherry-picking/publishing to OCP 3.4, Online, and Dedicated in the morning (RDU time). I'll send a note out to some lists when those are published and redirects from the old |
@openshift/devex fyi, our docs have been significantly refactored. |
Preview build can be found here: https://ci.openshift.redhat.com/openshift-docs-master-testing/latest/dev_guide/builds/ |
There is some Customer Portal maintenance going on today which is blocking doc publishing there, so I'm holding off publishing this in OCP 3.4 docs (on both docs.openshift and the Customer Portal) until that's settled down. |
Trello card: https://trello.com/c/sG2J1yry/413-builds-section-improvements
Latest WIP preview:
http://file.rdu.redhat.com/~adellape/021417/buildsfinally/dev_guide/builds/
Open PRs to consolidate: