[Documentation Clarity] Please clarify when specific fields are added to the configuration schemas #9151
Comments
|
My opinion is that we always describe in which version something was added when we add something to the documentation (including the kops api). I try to do this with my PRs, but we have no routine for this. It would be cool to have badges similar to k/k, but I think adding it in text is a good start. It is worth to mention that one should in no way rely on docs outside of master. Anything in those docs are updated purely based on when we do docs changes as part of backported features. Other changes to the docs are not backported. |
If one can't rely on docs outside of master, one can't run master in production, and the docs in master can't be relied upon for the versions which users do run in production, then which docs, pray tell, are users supposed to rely upon?! |
|
Users are supposed to use http://kops.sigs.k8s.io at least. But it is sort of "broken" when sections don't have any version info in them. |
|
My question about the proposal to have the documentation describe the behavior of multiple versions of kops simultaneously is "What is the documentation deprecation policy?" My PR #8372 to remove the documentation of the Calico CIDR migration that happened around kops 1.7 has languished for months. The documentation is going to be difficult to read and maintain if it has to cover all the variations of behavior that happen over a wide range of releases. |
|
So I did some research:
Does anyone want to look into either of those last two items? |
|
@rifelpet I can give the structor tool a go and see how it works! |
|
/assign |
|
Not really that confident we'll be able to manage multiple versions of the docs. But go for it. Personally, I think the widget is more helpful, and should be used by versioned docs anyway. On deprecations, I have removed most that I don't find useful anymore. But old release notes ... I don't see the harm of keeping those around at least. I am more concerned where users can be confused. |
|
I think the main "gap" in versioning docs with our release branches is when a new feature is cherry-picked from master to a release branch but the associated docs are not. I don't know how often that happens, since usually the docs are included in the same PR as the feature itself. I suppose we'd need to decide on a policy for all of the non-feature information in the docs. Off the top of my head these are important pieces of information that are typically only updated in the master branch and never cherry-picked to release branches:
I'm sure there are others I've missed. If we want to encourage users to use docs from release branches we may need to consider cherry-picking some of that information. Regarding structor tool, I think we really only got the mkdocs and netlify setup working in kops 1.16 so it may be difficult or impossible to have structor build pages from versions before that. I guess we'll see how the tool handles that. |
|
It happens quite frequently. At least with the recent rewrites I did of various sections. "backporting" those would make me dread doing more of those (and there are more needed). Somewhat related: we should avoid backporting features. That would make the amount of backported docs a lot easier. |
Question: how tightly coupled is the version of Kops to the version of Kubernetes to be managed, really? In spite of what the docs claim, we (and I imagine, most others) will run the version of Kops that corresponds with the version of Kubernetes that's installed in the cluster. If Kops 1.17.0 really supports Kubernetes 1.16.x and 1.15.x, then why bother maintaining older release branches at all? Why not tell people running earlier versions of Kubernetes to just run the latest Kops? |
|
In short https://kops.sigs.k8s.io/welcome/releases/ is really accurate. And to some extent, we really only support the latest version of kops unless something really critical happens (security or things like etcd-manager certs). |
|
Often times a new Kops minor release introduces additional changes that a user may not be comfortable with applying at the moment. For example, new Kops minor versions often change CNI provider manifests. By providing bug fixes to older Kops minor versions we can allow users to make those broader changes like CNI upgrades at a time that is more convenient for them. Regarding backporting features, I agree thats something that we should avoid. I think it will get better as Kops gets caught up with k8s releases, and 1.19's upstream delay will be a good opportunity for that to happen. We may have more features backported to release-1.18 before 1.18 is released as stable, so theres an opportunity for docs inconsistencies there. For 1.19+ I'm hoping that Kops will be "caught up" to the desired delay behind k8s, and the amount of cherry-picks will be greatly reduced. |
|
Quick update on structor - I just tried to give it a first try but seems like it has a hard requirement for the |
|
👍 one issue we'll have with structor is that netlify only clones the repo with the master branch, so we'll need to add a remote and fetch the other branches that we want structor to checkout. a luckily we're only dealing with a public repo, so we don't need to deal with ssh keys or GH deploy tokens: https://community.netlify.com/t/checkout-a-second-branch-during-build/6834 |
|
Gotcha, yeah that makes sense! |
|
i see netlify.toml has a single |
|
Sounds good to me! |
|
So I started looking into adding the feature stability widget with
git log --pretty=format:"%H" -S'you can enable NodeLocal DNSCache' -- docs/cluster_spec.md
git branch -r --contains f754cbda7d81df2848de017754587ba9acb4a9fe | grep origin/release
It could probably work in most cases, but feels a bit hacky to me, and I was wondering if there's a better/easier way to accomplish this task. I'd love to get some feedback (even a 👍 would be very helpful so I'll know I'm in the right direction) before I start working on this. |
|
I think going into the gitlog would be quite error prone. In many cases docs and feature have been committed independently. Then you have backports that may or may not include the docs. |
|
Agreed I think manually defining the versions for each feature is a sufficient first step. the macro could be used like: ## NodeLocal DNS
{{ kops_feature_table(kops_added_ff='1.16.0', kops_added_default='1.18.0', k8s_min='1.16.0') }}
NoteLocal DNS is ...we'd have to decide the relevant versions that we want displayed for a feature which correlate to arguments for the macro function. some might be optional in which case we just omit that set of table cells. I was thinking:
any others? |
|
Thanks for these comments! def kops_feature_table(kops_added_default, kops_added_ff=None, k8s_min=None):
optional_args = {
"Feature Flag": kops_added_ff,
"Minimum Required K8s Version": k8s_min
}
response = f"Stable: {kops_added_default}"
for arg in optional_args.keys():
if optional_args[arg]:
response += f", {arg}: {optional_args[arg]}"
return response |
|
I'm looking for examples in the k8s docs that we could mimic. I see references like this and this
But we need to display more information than that. Here are some ideas I came up with: bulleted list: renders as: FEATURE STATE:
no list: renders as: FEATURE STATE: horizontal table: renders as: FEATURE STATE:
vertical table: renders as:
We'd also want to see how those render with the site's theme rather than github's markdown renderer here. We can also go further by rendering html directly and bundle some CSS with the site to reduce the amount of vertical space on the page that this information uses. Lastly I wonder if it's helpful to mention the feature flag name in the |
|
Great examples there. Think the vertical table is my favourite. |
|
Thanks again for the useful feedback! def kops_feature_table(kops_added_default, kops_added_ff=None, k8s_min=None):
table = ["| FEATURE STATE: | |",
"| -- | -- |",
f"| Alpha (Feature Flag) | Kops {kops_added_ff} |",
f"| Default | Kops {kops_added_default} |",
f"| Min K8s Version | {k8s_min} |"]
for line in table:
if "None" in line:
table.remove(line)
return "\n".join(table)And called it from the Seems like the end result could use some cosmetics though:
Side note on the macro function itself- you can see that I initially create it as a Same call to the macro, this time without providing the Results in this:
|
|
Looks cool to me. Great work! I wonder if perhaps horizontal is more user friendly as it doesn't consume that much estate between header and text. |
|
Sorry it took me so long to get back to that, but I've finally found some time to do this 😄
The only change here aside from the alignment is that I decided to keep WDYT? Which one looks better? |
|
I love it! |
|
Same for Default. If none, best to omit the column. |
|
Done!
In order to add this table to every place in the docs, we just have to add this line (with the desired values): So in our example: #### Enabling Encryption in Cilium
{{ kops_feature_table(kops_added_default='1.19', k8s_min='1.17') }}
As of Kops 1.19, it is possible to enable encryption for Cilium agent.
In order to enable encryption, you must first generate the pre-shared key using this command:And here's the function that generates the table: @env.macro
def kops_feature_table(kops_added_default=None, kops_added_ff=None, k8s_min=None):
# Generate the title line based on the provided variables.
# Start with the a title to indicate this is a feature state table.
# Then, start with a | to open the table column and concatenate every field if its matching variable exists.
title = '** FEATURE STATE **\n\n|'
if kops_added_ff:
title += ' Alpha (Feature Flag) |'
if kops_added_default:
title += ' Default |'
if k8s_min:
title += ' Minimum K8s Version |'
# Add a matching number of column separators to the title (sum the count of | and remove the closing one)
separators = '| :-: ' * (title.count('|') - 1)
separators += '|'
# Generate the values line based on the provided variables.
# Same logic as the title line
values = '|'
if kops_added_ff:
values += f' Kops {kops_added_ff} |'
if kops_added_default:
values += f' Kops {kops_added_default} |'
if k8s_min:
values += f' {k8s_min} |'
# Create a list object containing all the table rows,
# Then return a string object which contains every list item in a new line.
table = [
title,
separators,
values
]
return '\n'.join(table)I realize it's a bit nasty and I'm trying to think if there could be a better way to achieve the same output. I'd be happy to get any suggestions on making the code cleaner. |
|
looks great! You could probably combine the sets of if statements: title = '** FEATURE STATE **\n\n|'```
values = '|'
if kops_added_ff:
title += ' Alpha (Feature Flag) |'
values += f' Kops {kops_added_ff} |'
...but feel free to open the PR now and we can iterate on it there :) |
1. What
kopsversion are you running? The commandkops version, will displaythis information.
1.16.2
2. What Kubernetes version are you running?
kubectl versionwill print theversion if a cluster is running or provide the Kubernetes version specified as
a
kopsflag.1.16.9
3. What cloud provider are you using?
AWS
4. What commands did you run? What is the simplest way to reproduce this issue?
N/A
5. What happened after the commands executed?
Silent dropping of configuration. See below
6. What did you expect to happen?
At least an error message
7. Please provide your cluster manifest. Execute
kops get --name my.example.com -o yamlto display your cluster manifest.You may want to remove your cluster name and other sensitive information.
See below
8. Please run the commands with most verbose logging by adding the
-v 10flag.Paste the logs into this report, or in a gist and provide the gist link here.
N/A
9. Anything else do we need to know?
We're in the middle of a cluster upgrade to Kops 1.16.2 and nearly suffered an outage because we tried to add fields like
InstanceGroup.spec.sysctlParametersandCluster.spec.networking.amazonvpc.envVarsthat are documented in themasterbranch, not accepted in1.16.2, and are silently dropped by1.16.2instead of throwing an error about unknown configuration.I understand that
masterreflects the newest state of development rather than the version of the documentation that is tied to a specific release, and that version-controlling the documentation in this way is beneficial precisely because it does allow me to see what the precise state of the documentation is for the version that I'm installing. Nevertheless, it is extremely easy to forget to set the tag on GitHub when viewing the documentation and presume that I am looking at the "correct" version of the documentation for what is the latest stable / certified / production-ready version of Kops. In many projects, the latest recommended version for installation is generally sufficiently close to if not identical tomasterthat the documentation drift does not present a problem in practice; in Kops, this is not the case, particularly as the manifest version is set toapi/v1alpha2and not changed when fields are added or changed.My recommendation is to adopt a similar standard as that found in the main Kubernetes user-facing documentation, which, even for the latest versions, clearly indicates in which version various features were made available. This ensures, for instance, that Kubernetes 1.16 users do not attempt to work with EndpointSlices. As such, as a user, I would be able to see from reading the docs in
masterthat various features are not yet available for the version of Kops that I am trying to install.The text was updated successfully, but these errors were encountered: