Installer Enhacement Proposal: Manifests from STDIN #575
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Changing state to "provisional".
|
|
||
| Modify the openshift-install binary to accept input from STDIN for all targets which consume manifests. For example: | ||
|
|
||
| - Create manifests from customized install-config.yaml: `kustomize build github.com/myorg/myBlueprint/install-config-kustomization | openshift-install create manifests -f -` |
There was a problem hiding this comment.
Is the intent to make it easier to pass extra manifests to the installer, or to make it easier to modify the manifests created by the installer?
There was a problem hiding this comment.
A bit of both - in the example, the yaml generated by the kustomize build would adhere to the current install-config scheme. A more mundane/contrived example would be openshift-install create install-config && cat install-config.yaml | openshift-install create manifests -f -
The idea is that we'd be supporting a simple and powerful pipeline model which could for instance ingest install-configs created by kustomize and streamed in through stdin.
However, we'd also like to define some new manifests - e.g. "machine-configs" to enable kernel modules, disable dhcp on interfaces, etc.
There was a problem hiding this comment.
I guess the source of my confusion is that I didn't really consider the install-config.yaml file as "manifest" in the same way that the files created by openshift-install create manifests. Would you expect to be able to pass the output of create manifests through kustomize?
There was a problem hiding this comment.
The idea would be to have a generic install-config.yaml describing at a high level view, how the cluster looks like (number of masters and workers, flavor of VMs, etc.) and use kustomize to overwrite specific information from the site (cluster name, domain, ipmi addresses, etc.). Using the base install-config.yaml + kustomize patch, the result shall be use to generate the installer manifests.
This approach will give organizations the ability to use a common recipe or install-config.yaml and customize only certain parameters. This would be very useful in edge computing usecases at scale.
There was a problem hiding this comment.
I agree I was thrown off by 'manifests' I assumed it was limited in scope to the output of openshift-install create manifests perhaps we should be more generic and speak of "Installer inputs" or "Installer State"?
There was a problem hiding this comment.
From kustomize v3.7.0 it's possible to create reusable "kustomizations" called Components.
Let's say that according to this enhacenment proposal, the output of openshift-install create manifests generates in the chosen directory the following structure:
- openshift/
- manifests/
- kustomization.yaml (this file will list all manifests as resources)
and we want to apply some remote overlay on top of the manifests generated by the installer. We would need to edit the generated kustomization.yaml with the following components section:
resources:
- manifests/openshift-config-secret-pull-secret.yaml
- manifests/machine-config-server-tls-secret.yaml
- manifests/kube-system-configmap-root-ca.yaml
- manifests/kube-cloud-config.yaml
- manifests/etcd-signer-secret.yaml
- manifests/etcd-serving-ca-configmap.yaml
[...]
- openshift/99_openshift-cluster-api_worker-machineset-2.yaml
- openshift/99_openshift-cluster-api_worker-machineset-1.yaml
- openshift/99_openshift-cluster-api_worker-machineset-0.yaml
- openshift/openshift-install-manifests.yaml
- openshift/99_role-cloud-creds-secret-reader.yaml
- openshift/99_kubeadmin-password-secret.yaml
- openshift/99_cloud-creds-secret.yaml
components:
- git::https://github.com/oglok/blueprint-test.git//overlay-manifests/
The manifests located in that remote location will look like:
- etcd-service-port.yaml
- etcd-host-service-port.yaml
- kustomization.yaml
etcd-service-port.yaml
apiVersion: v1
kind: Service
metadata:
name: etcd
namespace: openshift-etcd
labels:
k8s-app: etcd
spec:
ports:
- $patch: replace
- name: etcd
port: 2378
protocol: TCP etcd-host-service-port.yaml
apiVersion: v1
kind: Service
metadata:
name: host-etcd
namespace: openshift-etcd
spec:
ports:
- $patch: replace
- name: etcd
port: 2378
protocol: TCP kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1alpha1 # <-- Component notation
kind: Component
patches:
- etcd-service-port.yaml
- etcd-host-service-port.yaml This can be rendered by kustomize. So we propose the installer to accept this kustomized output as input:
kustomize build generatedManifests/ | openshift-install create cluster
There was a problem hiding this comment.
@dhellmann do you want me to add this example to the enhancement too?
There was a problem hiding this comment.
@oglok I think having a detailed example in the body of the enhancement would be a good addition.
One of the technical challenges I see with implementing this is that, as far as I know, the installer treats manifests as files rather than individual resources. Since a file can contain multiple resources, and the names of the files are both significant and not necessarily derived from the contents in an obvious way, it will be challenging to turn a single input stream from stdin into the right artifacts to replace the cached versions on the filesystem. Which is not to say it can't be done, just that it isn't obvious to me how to do it. Have you started looking at the implementation of this idea already?
Reviewing the way that kustomize is used in the cluster-baremetal-operator repository, I see that it is configure to create output files in a named directory instead of streaming the results to stdout. Using that approach may make it easier to integrate with installer. A user would run create manifests, then kustomize with the manifests directory as input, then create cluster.
There was a problem hiding this comment.
Ok, I will add the example to the enhancement.
We haven't started looking at the implementation, but after one of the meetings we had with @crawford , he said it should be doable.
There was a problem hiding this comment.
In order for this to work, the installer would need to start keying assets based on their type and name rather than their on-disk path (since there aren't any paths when reading assets from STDIN). There's probably some annoying gotchas in this approach, but it doesn't fundamentally alter the design of the installer.
| reviewers: | ||
| - "@fzdarsky" | ||
| approvers: | ||
| - "@crawford" |
There was a problem hiding this comment.
@dgoodwin too, I'm not sure if this would have value for Hive.
There was a problem hiding this comment.
Do you want me to add them as reviewers? or approvers?
|
|
||
| ## Motivation | ||
|
|
||
| Infrastructure-as-Code (e.g. GitOps) is an operational best practice for provisioning and managing large numbers of clusters and services. A typical IaC workflow involves pulling Kubernetes resource manifest templates of a specific version from a `git` repo, rendering those templates for a given cluster instance, and then running `openshift-install` (resp. `oc apply`) on the rendered manifests to install (resp. update) a cluster. |
There was a problem hiding this comment.
I don't claim to be a GitOps aficionado, so perhaps this is a naive question, but what precludes leaving this entirely outside of the purview of the Installer? Would you not want to source control the actual rendered assets that were used rather than the template and mutations? What if the actual manifests created by openshift create manifests change over time given the same install-config.yaml input? Is that desirable or undesirable regardless of whether or not it breaks your pipeline.
There was a problem hiding this comment.
Both approaches (version controlling the one-off generated manifests vs the install-config.yaml + mutations) are valid and used, each with pros and cons.
In both cases, users would want to pin versions of all installation artefacts (openshift-install, kustomize, ...) matching the to-be-generated/consumed manifests, such that re-deploying the same git tag (e.g. for disaster recovery) at a later point in time yields the exact same result. In both cases, changes to newer installer versions may require updates to manifests.
One advantage of the "install-config + mutations" approach with kustomize is that it minimizes the delta that needs to be (version) controlled. So even when manifests created by the create manifests target drift (it's not a stable API after all), the "patches" are more likely to apply cleanly still.
|
|
||
| Modify the openshift-install binary to accept input from STDIN for all targets which consume manifests. For example: | ||
|
|
||
| - Create manifests from customized install-config.yaml: `kustomize build github.com/myorg/myBlueprint/install-config-kustomization | openshift-install create manifests -f -` |
There was a problem hiding this comment.
I agree I was thrown off by 'manifests' I assumed it was limited in scope to the output of openshift-install create manifests perhaps we should be more generic and speak of "Installer inputs" or "Installer State"?
|
|
||
| #### Resources created by the installer | ||
|
|
||
| The `openshift-install create manifests` will create a kustomization.yaml file that will list the resources located in the openshift and the manifest directories. The content of the file should look like this: |
There was a problem hiding this comment.
Can we toggle this functionality based on the presence of a global flag? e.g. openshift-install --kustomize create manifests
There was a problem hiding this comment.
Sure, I think a global flag in order to trigger the creation of the kustomization.yaml file makes sense. I'll add that.
|
|
||
| ### Implementation Details/Notes/Constraints | ||
|
|
||
| The installer validates if the input stream contains all the necessary objects required for the installation process. This will ensure that the customization process has not created corrupted objects or non-valid manifests. |
There was a problem hiding this comment.
I might be mistaken, but I thought the installer validated all provided assets. I assume that this new STDIN mechanism would just be another way of providing assets (and a user might make use of both simultaneously), so objects provided through this would be validated just the same. In fact, can you add some detail about how the installer should treat assets provided via the filesystem versus STDIN. My preference would be for us to treat both equally. This also means that assets can similarly be provided "out of order", resulting in the following warning:
Discarding the <asset name> that was provided in the target directory because its dependencies are dirty and it needs to be regenerated
There was a problem hiding this comment.
I think your comment is totally understandable based on the proposal. My intention here was to leverage the validation already made by the installer. We don't want that validation to be different from what it exists. I'll rephrase this.
|
|
||
| Modify the openshift-install binary to accept input from STDIN for all targets which consume manifests. For example: | ||
|
|
||
| - Create manifests from customized install-config.yaml: `kustomize build github.com/myorg/myBlueprint/install-config-kustomization | openshift-install create manifests -f -` |
There was a problem hiding this comment.
In order for this to work, the installer would need to start keying assets based on their type and name rather than their on-disk path (since there aren't any paths when reading assets from STDIN). There's probably some annoying gotchas in this approach, but it doesn't fundamentally alter the design of the installer.
Signed-off-by: Ricardo Noriega <rnoriega@redhat.com>
|
This is an useful feature when some components need to be configured in a different way than the default one, and can cause a failure on the installation or race conditions if we rely on day 2. |
|
@dhellmann @sdodson if you're satisfied with the most recent changes, what can we do to move this towards acceptance? |
There was a problem hiding this comment.
I am not convinced that there is overall much added value to the installer to support piping from STDIN. I think it introduces a number of grey areas. I have not seen use cases yet where streaming from STDIN makes the installation process significantly easier.
|
|
||
| ## Summary | ||
|
|
||
| In many occasions, users need to customize the install-config target and/or the manifests generated by the OpenShift Installer in a reproducible manner. The most popular tool to allow this type of customization is “Kustomize”, and it is already well-known by the Kubernetes ecosystem. The result of any “Kustomization” is sent to standard output as a stream of manifests. |
There was a problem hiding this comment.
The output does not have to be sent to standard output. What prevents the user from sending the output to the existing manifests directory?
There was a problem hiding this comment.
The problem is openshift-install's assumption on file structure and naming when reading from the manifest directory.
Popular templating tools (kustomize, ksonnet, helm, ...) either output a single stream of manifests to stdout (from where it could be redirected to a single file, of course) or to a manifest directory, but then it's up to the tool how files get named (typically some string concatenation of the resource's GVKN) and organized into subdirs (typically: flat).
Means one cannot currently solve the common use case of templating manifests without additional scripting that reorganizes a tool's output to meet openshift-install's requirements.
There was a problem hiding this comment.
Then let's consider as an alternative whether openshift-install can work without some of those assumptions on file structure and naming. If openshift-install is going to support streaming from STDIN, then it would need to rid itself of those assumptions for that input path anyway. So this is not a compelling argument to me to allow for reading from STDIN.
|
|
||
| Modify the OpenShift Installer to accept inputs from STDIN for all targets. For example: | ||
|
|
||
| - Create manifests from customized install-config.yaml: `kustomize build github.com/myorg/myBlueprint/install-config-kustomization | openshift-install create manifests -f -` |
There was a problem hiding this comment.
Accepting the install-config.yaml from STDIN is going to be confusing. If the user runs create cluster -f - with an empty directory, is the expectation that the STDIN is going to be the install config or additional manifests? Does the answer change if the directory already has an install-config.yaml?
I don't see enough value add of this to convince me to carry code to support this when it is pretty easy to do the following instead.
kustomize build https://github.com/myorg/myblueprint.git//install-config/ > $TMPDIR/install-config.yaml
openshift-install --kustomization create manifests --dir=$TMPDIR
There was a problem hiding this comment.
The expectation is that the installer can read manifests from stdin like it currrently can read from a --dir. The current proposal is to express this to use -f - as argument, but if something like --stdin makes more sense, that'd be perfectly fine.
Just like the installer doesn't know a priori whether the --dir contains an install-config.yaml or additional manifests, it wouldn't know reading from --stdin. The assumption we've made is that once the resources read from stdin are mapped back into the same data structure when reading from --dir, the following logic would be identical.
Re openshift-install --kustomization create manifests --dir=$TMPDIR: This could work and was in fact part of an earlier proposal. However, we've been given the guidance by the architects not to propose an approach that would make the installer opinionated on a given templating tool or even to expect kustomize libs to be added to the installer. This is why we're proposing the stdin approach now.
There was a problem hiding this comment.
There is more in the assets directory than just manifests. Is your expectation that everything in the assets directory is going to be sent in via STDIN? Or is, for example, the asset store going to remain in the assets directory?
You are correct that the installer does not know a priori whether the assets director contains and install-config.yaml or additional manifests. The installer builds the asset graph up to the targets desired in the command and tries to load from the asset directory whatever assets are needed. The installer relies on the names of files to ascertain whether the files for a particular asset are on disk. It is not clear to me how the "mapp[ing] back into the same data structure" is supposed to work without the file names. Let's take the ignition config assets as an example. How is the installer expected to differentiate between the master ignition config and the worker ignition config when read from STDIN?
Re openshift-install --kustomization create manifests --dir=$TMPDIR: You can ignore the use of the --kustomization flag here. That was lifted from on of the use cases later in the document. It has no bearing on the behavior of the installer as it relates to input.
|
|
||
| ## Proposal | ||
|
|
||
| Modify the OpenShift Installer to accept inputs from STDIN for all targets. For example: |
There was a problem hiding this comment.
I do not think that we should accept STDIN for all targets. I think we should be deliberate. If the user wants to add additional manifests from STDIN, they must call create manifests with their additional manifests. Or perhaps even have a separate target that they would call that sits after create manifests and before create ignition-configs. But then I would argue that the user could just as easily place the files in the manifests directory themselves without involving the installer.
There was a problem hiding this comment.
Just to clarify: It is not only about adding manifests, but also changing installer-generated manifests through config overlays or templating. And I agree the installer doesn't have to / shouldn't be involved how a user achieves this. But the installer can facilitate this by offering some way of reading manifests without mandating a file naming and directory structure convention.
There was a problem hiding this comment.
I don't follow how your argument relates to whether we accept STDIN for all commands or only for the create manifests command.
|
|
||
| The syntax and workflow will be similar to `oc apply -f -`. | ||
|
|
||
| A straight example of a use case is explained next. From Kustomize v3.7.0 it's possible to create reusable customizations called `Components`. Acording to this enhancement proposal the output of `openshift-install --kustomization create manifests` will generate the following structure in the chosen directory: |
There was a problem hiding this comment.
The --kustomization flag is mentioned here for the first time without any introduction into what the flag does.
There was a problem hiding this comment.
Thanks for pointing this out. We added this as response to comment #575 (comment) to add a global flag to enable/disable the behavior of generating a "kustomization.yaml".
We need to further clarify this.
There was a problem hiding this comment.
The --kustomization is not right here. --stdin would be a better fit, since we don't want to be opinionated on the tool that the user must leverage to perform customizations on installer inputs.
| Modify the OpenShift Installer to accept inputs from STDIN for all targets. For example: | ||
|
|
||
| - Create manifests from customized install-config.yaml: `kustomize build github.com/myorg/myBlueprint/install-config-kustomization | openshift-install create manifests -f -` | ||
| - Create ignition files from customized set of manifests: `kustomize build ~/myBlueprint/01_manifests | openshift-install create ignition-configs -f -` |
There was a problem hiding this comment.
Is the expectation that the manifests from STDIN will supplement the manifests that already exist in the assets directory? If so, I assume that the manifests from STDIN will have precedence over the manifests in the assets directory if there are conflicts. How will the manifests from STDIN maintain run order in the manifest file name? For example, if STDIN yields replacements for the worker machinesets, is the expectation that the installer will somehow retain the 99 run order prefix on the machinesets manifests?
There was a problem hiding this comment.
We were assuming input is either from stdin or from disk. Mixing both is an interesting idea, though. Yes, I think making stdin take precedence would make sense in this case.
Once resources read via stdin are mapped back (via the GVKN) to the installer's internal data structure, the run order would be identical to the installer's default for any resource known to the installer (resp. generated by it). For additional resources, one way could be to simply default to a 99 run order (resp. order them last). Another could be to sort additional manifests in according to the order in the stdin stream.
|
|
||
| #### Resources created by the installer | ||
|
|
||
| The `openshift-install create manifests` will create a kustomization.yaml file that will list the resources located in the openshift and the manifest directories if the flag `--kustomize` is passed. The content of the file should look like this: |
There was a problem hiding this comment.
I would rather not have the installer own code around supporting a kustomize.yaml file, particularly when the list of resources is simply all of the files.
(echo "resources:"; find "${TMPDIR}" -type f -not -name '\.*' -printf '- %P\n') > "${TMPDIR}/kustomization.yaml"
Thank you for your review, @staebler! I hope I could shed some light on the added value in the responses to your detailed comments. Gist is that the installer imposes convention on file naming and directory structure that tooling for config overlays/templating popular with GitOps and Infrastructure as Code isn't aware of. Therefore, users currently need to implement their own logic how to adapt one to the other and our own engineering and field teams have created lots of snowflake solutions for this. |
Can you give me some concrete examples of installer-imposed conventions that are onerous for GitOps and Infrastructure as Code? Here are the only conventions that I am aware of that the installer imposes for manifests.
Would it be helpful if the installer read recursively through the manifests and openshift folders? |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
|
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
|
@openshift-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
This OpenShift enhancement proposes to allow the installer to read a stream of manifests from STDIN on the main targets that consume manifests.