Do we need to re-inject this later? Will it impact anything?

cc @lavalamp

xref discussion in #37055 (comment)

The presence of this means we can't fast-forward release branches while they're still tracking master, which is really unfortunate, and requires a build/update-openapi-spec invocation just to fast forward the branch :-/

See kubernetes/sig-release#845

/lgtm
/approve

/hold
(feel free to lift, if there are no objections)

/sig release
/area release-eng

/priority important-soon

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justaugustus, liggitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

/test pull-kubernetes-e2e-gce-device-plugin-gpu

Same question as @cblecker -- why is this here? Do we need to add it back in some later step in some pipeline?

It's used at https://github.com/kubernetes-sigs/reference-docs/blob/master/gen-apidocs/generators/api/open_api.go#L161, which is really unfortunate, because it is misleading/inaccurate (for example, the current swagger.json at master claims it is for v1.18.0, and the currently published docs for 1.16 claim they are for v1.17.0)

can we update the release branch once when we cut the branch or to make easier when we stop doing the branchff ?

@liggitt Sorry for jumping in too late, hopefully not toooo late? We will check the API reference doc generation logic if necessary -- maybe we were using the wrong branch/tag in the Makefile/script?. However, tagging the version as "unversioned" may make tracking the changes from one version to another version very difficult. I'm wondering if there are some compelling reasons to do this other than the API reference docs? I'm more inclined to have the API spec versioned. The version string doesn't look to me like the major blocker for fast forwarding? If there are changes to the API spec, we are supposed update the version string anyway. If there is no change to the API spec, why are we blaming the version string?

That file is the only difference between master and the release branch when we're in the fast forward period of a release, because as soon as the release branch is created, its swagger file contains 1.x.y, and master's contains 1.x.y+1. This was the only reason we could not simply fast forward the release branch with git before lifting code freeze on master.

The current swagger generation does not base the version on whether there was an API change or not, but on the nearest tag. API changes can merge during development and do not modify the version in the static swagger file.

@liggitt Thanks for the response. If API changes can be merged without modifying the version in the swagger file, where is the source of truth then? It means we cannot tell which version a swagger file is for; it means my 1.17.1 spec may differ from your 1.17.1.
Based on my study, API spec changes since (at least) 1.13 have been well managed. I did investigated the diffs between 1.13 and 1.14, 1.14 and 1.15 ... 1.16 and 1.17.
Maybe we should instead tune the version changing/locking process somehow? If the sole inconvenience about the code freeze period, maybe we should/can keep the spec version at 1.x.y till the moment the the code freeze is lifted? We bump the version at master only after the end of the code freeze window?
I'm not an expert on the release process. Please bear with me if I am asking stupid questions. :)

where is the source of truth then

the git commits/tags are the source of truth

for example, to see the diff between 1.16.0 and 1.17.0:

diff \
  <(git show v1.16.0:./api/openapi-spec/swagger.json | jq .definitions) \
  <(git show v1.17.0:./api/openapi-spec/swagger.json | jq .definitions)

Okay. The incorrect API version string on the website was introduced in kubernetes/website#16569, for some unknown reasons. I'm fixing it in kubernetes/website#18587.

I'm fixing it in kubernetes/website#18587.

sounds good, thanks

@liggitt Even with PRs submitted to fix the version for 1.16, I'm still against tagging the swagger spec as "unversioned". An API spec must have its own version, which is independent of the git tag. We must have other ways to address this.
I have no idea how files in kubernetes/website#16569 were generated but it was an error, not supposed to happen. We are also working on improving the Makefile/scripts for generating API refs so that people won't make such mistakes again.
Again, if the reason behind this PR is really about the incorrect version string in 1.16 version of API docs, I'd suggest we revert this series of changes because my gut feeling is that we are on the wrong direction.

The reason for this PR is that it deduplicates information that is already available via git tags, and fixes the issue with master/release branch fast forwarding.

An API spec must have its own version, which is independent of the git tag. We must have other ways to address this.

When fetched from a running API server (which is the recommended way to obtain the openapi schema for a particular cluster), the openapi document contains the server's version, and is appropriate to make use of. The static file checked in here is for convenience only, and the git sha/tag should be used to determine what version the file is for.

@liggitt Revisiting this statement:

Does this PR introduce a user-facing change?:

NONE

It's entirely possible that there shouldn't be a user impact, but there clearly is.

I'm curious: how did you arrive at this assessment? Did you reach out to other SIGs to assess impact? Given the timing of KubeCon and the holidays, it's possible SIG Docs could have missed any outreach. I want to make sure we're not missing collaboration attempts.

Given the importance of community collaboration, is it reasonable to ask for a KEP and/or possible reversion?

EDIT: No need to revert prior to thoroughly excavating the docs generation process, which is admittedly brittle.

That was oriented at users consuming Kubernetes releases. This does not change the behavior of the API server or other components, or any APIs we produce.

Hi, @liggitt ,

When fetched from a running API server (which is the recommended way to obtain the openapi schema for a particular cluster)

Would this be the easiest way (eliminate guesswork so that this can be automated) to generate a valid Open Api spec that corresponds to a tagged release version?
What if I want to build the docs before the release is released -- just to check that things actually build and dependencies are checked, etc.? What branch could be used?
Clearly it is important to validate that the version of the generated spec matches the release otherwise the docs are unfortunately displaying incorrect results.

Would this be the easiest way (eliminate guesswork so that this can be automated) to generate a valid Open Api spec that corresponds to a tagged release version?

The easiest way to get the generated spec for a tagged release version is to fetch the file at that tag, e.g. git show v1.16.0:./api/openapi-spec/swagger.json

What if I want to build the docs before the release is released -- just to check that things actually build and dependencies are checked, etc.? What branch could be used?

The release branch, e.g. git show release-1.16:./api/openapi-spec/swagger.json

Clearly it is important to validate that the version of the generated spec matches the release otherwise the docs are unfortunately displaying incorrect results.

We have a verification script that ensure the definitions/schemas/operations/etc in the committed file matches the output of the server at that sha.

@liggitt . Thanks for the reply. Still digesting the info and previous history.
If I don't run the API server (for example, from the v1.17.0 branch) to obtain the spec, I can get a copy of the spec for the tagged release (as shown above). I would expect that the generated file from the API server and the static file are exactly the same. Or is the info.version field missing in the static file?

I would expect that the generated file from the API server and the static file are exactly the same.

Making them exactly the same causes the various problems described above (and in the PR that introduced the version to the static file):

• drift between master/release branches during fast-forward period
• verification script errors on release-tag commits (tagging a commit changes the content of what the API server built from that commit serves, but that changed content cannot then be added to the static file in that commit)

To summarize:

• when obtaining the file via git, the git branch/tag you use identifies the version
• when obtaining the file via the API server, the version is included in the served content

Got it. Saving the release-tagged spec is helpful (not sure who/what uses this file besides docs), but perhaps providing a build target, to generate the spec (run the API server), is all that is needed. The file can be generated on demand and with the correct version state.

@liggitt I've noticed that latest tags has the version undefined, for example:

$ curl -s https://raw.githubusercontent.com/kubernetes/kubernetes/v1.15.9/api/openapi-spec/swagger.json | jq .info.version
"unversioned"

so I guess this static spec will always be "unversioned" (in branches and tags) and we have to update version using the tag name. Correct?

BTW, kubernetes-client/* are probably affected.