Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix Operation names for subresources #49357

Merged
merged 2 commits into from
Jul 25, 2017
Merged

Fix Operation names for subresources #49357

merged 2 commits into from
Jul 25, 2017

Conversation

mbohlool
Copy link
Contributor

@mbohlool mbohlool commented Jul 21, 2017 •
edited
Loading

Subresources may have their kind but in operationID and description we should use their parent kind not their own kind. The subresource name represents subresources and no need to repeat in with their kind. e.g. Scale has kind of Scale and subresource name of Scale. This creates names like read scale of the specified Scale or readCoreV1NamespacedScaleScale for scale on replication controller. This PR fixes description and operation ID for them to read scale of the specified ReplicationController and readCoreV1NamespacedReplicationControllerScale.

@caesarxuchao I would like this to be patched in 1.7 too as it is useful for all clients and there is no need to wait for 1.8 for this documentation fix.

Fixed OpenAPI Description and Nickname of API objects with subresources

fixes #49250

@mbohlool mbohlool added this to the v1.7 milestone Jul 21, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 21, 2017
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Jul 21, 2017
@janetkuo
Copy link
Member

This LGTM. Need to fix verification:

/go/src/k8s.io/kubernetes/federation/docs/api-reference is out of date. Please run hack/update-federation-api-reference-docs.sh

@caesarxuchao
Copy link
Member

cc @wojtek-t our 1.7 patch branch manager for the cherrypick request.

@janetkuo
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 21, 2017
@janetkuo
Copy link
Member

/retest

@janetkuo janetkuo mentioned this pull request Jul 21, 2017
Merged
@lavalamp
Copy link
Member

/approve

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 22, 2017
@wojtek-t
Copy link
Member

/retest

@wojtek-t
Copy link
Member

Will create a cherrypick once this PR is merged.

@janetkuo
Copy link
Member

/retest

@janetkuo
Copy link
Member

Looks like you need to re-run some auto-gen scripts

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 25, 2017
@janetkuo
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 25, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: janetkuo, lavalamp, mbohlool

Associated issue: 49250

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@janetkuo
Copy link
Member

/test pull-kubernetes-kubemark-e2e-gce

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 7249435 into kubernetes:master Jul 25, 2017
@wojtek-t
Copy link
Member

@caesarxuchao @mbohlool - it's not possible to automatically cherrypick it. If you want this cherrypicked, can you please prepare a cherrypick PR?

@mbohlool mbohlool mentioned this pull request Jul 25, 2017
Merged
@wojtek-t wojtek-t added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jul 26, 2017
k8s-github-robot pushed a commit that referenced this pull request Jul 26, 2017
Merge pull request #49592 from mbohlool/automated-cherry-pick-of-#493…
e5bc152 
…57-upstream-release-1.7

Automatic merge from submit-queue

Automated cherry pick of #49357 upstream release 1.7

Cherry pick of #49357 on release-1.7.

#49357: Fix Operation names for subresources
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.7" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

mbohlool added a commit to mbohlool/client-python that referenced this pull request Jul 26, 2017
@mbohlool
update generated client to get kubernetes/kubernetes#49357
387f5b5
This was referenced Jul 26, 2017
Merged
Closed
k8s-github-robot pushed a commit that referenced this pull request Jul 28, 2017
Merge pull request #49238 from janetkuo/apps-v1beta2-rs
7056b5b 
Automatic merge from submit-queue (batch tested with PRs 49238, 49595, 43494, 47897, 48905)

Add apps/v1beta2.ReplicaSet

~Depends on #48746~ (merged)
~Depends on #49357~ (merged)
xref: #49135

```release-note
Add a new API object apps/v1beta2.ReplicaSet
```
mbohlool added a commit to mbohlool/client-python that referenced this pull request Aug 11, 2017
@mbohlool
update generated client to get kubernetes/kubernetes#49357
b4f852a
@smarterclayton
Copy link
Contributor

smarterclayton commented Sep 9, 2017 •
edited
Loading

@Kargakis @deads2k this breaks openshift when introduced because we get this error:

F0909 13:11:59.007560   77320 openapi.go:38] Failed to register open api spec for root: Duplicate Operation ID createNamespacedDeploymentConfigRollback for path /oapi/v1/namespaces/{namespace}/deploymentconfigrollbacks and /oapi/v1/namespaces/{namespace}/deploymentconfigs/{name}/rollback.

It looks like we need to fix this to handle the case where the subresource name and the subresource kind are the same (i.e. DeploymentConfigRollback is the kind and has a top level representation, and there is a subresource of DeploymentConfig called rollback). If there's any open api aggregation going on, this would break as well, so the backport would need that fix as well.

@mbohlool
Copy link
Contributor Author

What would you suggest the name should look like in this case?

@0xmichalis
Copy link
Contributor

fwiw we should have deprecated/removed DeploymentConfigRollback some time ago @mfojtik @tnozicka

@smarterclayton
Copy link
Contributor

Subresources aren't the same as regular resources, so some separation between the core resource and all sub resources would be appropriate.

@smarterclayton smarterclayton mentioned this pull request Sep 15, 2022
Merged
bh717 pushed a commit to bh717/python-dapp that referenced this pull request Apr 1, 2024
update generated client to get kubernetes/kubernetes#49357
b5b6f70
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janetkuo janetkuo janetkuo approved these changes

Assignees

@janetkuo janetkuo

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.

Open API generates conflicting Operation ID
10 participants
@mbohlool @janetkuo @caesarxuchao @lavalamp @wojtek-t @k8s-github-robot @k8s-cherrypick-bot @smarterclayton @0xmichalis @k8s-ci-robot