-
Notifications
You must be signed in to change notification settings - Fork 1.9k
updates to the custom install draft #13468
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
Conversation
wking
left a comment
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 haven't looked at the text you're removing, but here are some suggestions for the text you're adding. Nothing seems particularly critical, so feel free to pick and choose whatever feedback that resonates ;).
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.
Some of this will probably be supported, see openshift/installer#1144. And the Kubernetes objects that we really care about should be managed (possibly indirectly) by the cluster-version operator. What is unsupported is turning off the cluster-version operator or otherwise overriding it. Pushing Kubernetes objects that do not fall under the CVO umbrella should be fine (e.g. creating and populating user-defined projects).
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.
@wking, I'll follow that PR and make more updates when it's merged.
Is there a good way for users to identify which Kubernetes objects are safe to change? I'm happy to add a section about modifying them back in, but only if we can specifically tell them how to confirm that what they're doing will be supported. (Product docs can discuss only supported scenarios.)
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.
Is there a good way for users to identify which Kubernetes objects are safe to change?
You can find out what the CVO manages by inspecting the update payload, but that doesn't cover indirect dependencies (e.g. objects pushed by CVO-managed operators). And it looks like Kubernetes considered and then rejected recording creator information. As long as they stick to namespaces/projects they create, they'll be fine. For namespaces we create and cluster-scoped objects, I'm not sure what an easy way would be to check whether a change would be supported short of making the edit and seeing if it gets stomped ;). Maybe @abhinavdahiya or @smarterclayton have some more elegant ideas.
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.
And it looks like Kubernetes considered and then rejected recording creator information.
Although creators can set ownerReference explicitly if they need to. Maybe we just need to push more operators to do that (if they aren't already).
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.
@jianlinliu, are you testing any modifications to Kubernetes manifests during installation?
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, we had some testing to modify Kubernetes manifests for customized install until some discussion in upstream repo - https://github.com/openshift/installer/pull/1088/files#r250505048. Since that, we lower the testing of any modifications to Kubernetes manifests, and any issue found during testing is not treated as a blocker issue any more. Because QE also thought modification via manifest and ignition is not officially supported.
I agree with you, if we support it, we have to tell user which Kubernetes objects are safe to change, or a better approach is to prompt the modification to install-config level, we treat everything in install-config is officially supported to allow modification.
@qinpingli pls follow up this discussion, make sure what we tested is on the same page like what we published on our official doc.
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.
Thank you @jianlinliu! @qinpingli, please check during the E2E tests and follow up. Per @vikram-redhat, we're merging frequently instead of holding docs for 2/22.
@sferich888, are we planning to support modifying Kubernetes manifests during installation at 4.0 GA?
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 not strictly true. For example, see here and surrounding discussion where we decided to include some properties in the wizard-generated install-config.yaml for which we have internal defaults. But the generated install-config.yaml certainly defers some configuration to later logic, so I dunno how much you want to discuss vs. glossing over at this stage. But I thought I'd point it out so you could make an informed decision, whichever way you go.
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.
Well. The description needs to be accurate here. If there's more nuance to the parameters that this file presents and what users can reasonably be expected to do with it, then I need to explain it somewhere. I need more info, though.
Which of the parameters that are in this "default" file but not set by the installation program can a user modify?
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.
Which of the parameters that are in this "default" file but not set by the installation program can a user modify?
Users can modify everything in the install-config.yaml, and if they choose invalid values (e.g. configuring zero masters), we'll exit with a validation error. But here are the three classes of parameters:
- Given in the wizard-created file, and...
- The installer has no default. For example,
baseDomainor the cluster name. The wizard will prompt you for these, but if you supply your owninstall-config.yamlwe'll error out if they aren't populated. - The installer can generate a default. For example, machine-pool
replicas. These are in the wizard-created file because it's convenient for the implementation, and there didn't seem to be any harm in that approach (more details in the discussion linked from my earlier comment in this thread). If you want, you can supply your owninstall-config.yamlwithout them, and the installer will happily inject its built-in defaults.
- The installer has no default. For example,
- Not given in a wizard-created file. For example, the
rootVolumemachine-pool configuration on AWS. The installer is able to generate defaults for these, but it does so in an asset that consumes the install-config, and not in the install-config asset itself.
|
Thank you @wking! After reading your comments, I have one immediate request. I'll make more updates to address your comments and add more information. |
kalexand-rh
left a comment
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 @wking! I've made more updates and have a few more questions and some commentary.
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.
@wking, I'll follow that PR and make more updates when it's merged.
Is there a good way for users to identify which Kubernetes objects are safe to change? I'm happy to add a section about modifying them back in, but only if we can specifically tell them how to confirm that what they're doing will be supported. (Product docs can discuss only supported scenarios.)
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.
That is a tongue twister. So, when we tell users to set up their domain or subdomain, is there extra guidance that we should give them? Like, you might not want to create separate subdomains for your different clusters because the route is a combination of the cluster name and domain?
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.
As long as that link doesn't change for GA, that's totally fair. (For GA, I'd like to change some of those links to the product 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.
Well. The description needs to be accurate here. If there's more nuance to the parameters that this file presents and what users can reasonably be expected to do with it, then I need to explain it somewhere. I need more info, though.
Which of the parameters that are in this "default" file but not set by the installation program can a user modify?
3a0157c to
d427cda
Compare
Updates to the customized install for https://jira.coreos.com/browse/OSDOCS-190
Preview build here: http://file.rdu.redhat.com/kalexand/012918/osdocs190/installing-aws/installing-customizations-cloud.html
@wking, will you PTAL? Because we only document supported changes, I've removed some of the information about changing Ignition Configs and Kubernetes objects that was in openshift/installer#1088