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.

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

Add deprecate description #127

Merged

Conversation

momom-i
Copy link
Contributor

@momom-i momom-i commented Mar 2, 2022

What type of PR is this?

/kind documentation

What this PR does / why we need it:

Once we change this simulator's frontend to request resource change directly to kube-apiserver, all APIs on server side can be deprecated.

Which issue(s) this PR fixes:

Fixes #95

Special notes for your reviewer:

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 2, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @momom-i. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 2, 2022
@sanposhiho
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 3, 2022
Copy link
Member

@sanposhiho sanposhiho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only two nits

docs/api.md Outdated
@@ -60,6 +60,10 @@ empty
| 202 | |
| 500 | something went wrong (see logs of the simulator server) |

---
## **deprecated APIs**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## **deprecated APIs**
## deprecated APIs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed: 4d227ee

docs/api.md Outdated
---
## **deprecated APIs**

The following APIs are deprecated. They will be removed in the near future.
## Apply Node
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you change all deprecated APIs titles like this?

Suggested change
## Apply Node
### Apply Node

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed: 4d227ee

@sanposhiho
Copy link
Member

I think you can resolve lint error with nolint like this.
//nolint: staticcheck // we need to use deprecated APIs for now.

Add this nolint on here:
https://github.com/kubernetes-sigs/kube-scheduler-simulator/blob/master/server/server.go#L24-L26

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 4, 2022
@momom-i
Copy link
Contributor Author

momom-i commented Mar 4, 2022

Thank you for your review and I'm sorry for ignoring error💦

Copy link
Member

@sanposhiho sanposhiho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
/lgtm
/approve

Let's wait #115
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 11, 2022
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 11, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: momom-i, sanposhiho

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 11, 2022
@sanposhiho
Copy link
Member

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 28, 2022
@k8s-ci-robot k8s-ci-robot merged commit 541a021 into kubernetes-sigs:master Mar 28, 2022
matthewygf pushed a commit to matthewygf/kube-scheduler-simulator that referenced this pull request Mar 28, 2022
* Add deprecate comment lines

* change some docs description

* fix indent and move not-deprecate APIs

* fix linter err
matthewygf pushed a commit to matthewygf/kube-scheduler-simulator that referenced this pull request Apr 18, 2022
* Add deprecate comment lines

* change some docs description

* fix indent and move not-deprecate APIs

* fix linter err
k8s-ci-robot pushed a commit that referenced this pull request Apr 18, 2022
…in (#121)

* Able to configure API server and port, thus expose it is possible via compose

* linters

* update property name to be more descriptive

* change the parameter arg to be more descriptive

* WIP - use create server chain to start k8s api server.

* WIP - remove unecessary shutdown fn

* setup annoymoususer for insecure testing

* update go mod sum

* instead of building kube-api , only do openapi-gen

* add vendor files into repo and keep existing openapi gen

* forgot to add openapi

* remove not needed generated api

* refactor due to linters

* change makefile to what it was for github action

* refactor due to cyclomatic complexity

* use original dockerfile step

* unnecessary if

* remove openapi step before test

* disable admission plugins to for node taints and service account checks

* Added docs to describe our kube-apiserver creation process and env var

* add permission doc

* Import from existing cluster (#111)

* feat:add ignoreRestart Option to Import on ExportService

* add:impl ReplicateExistingClusterService

* add:ExternalKubeClientCfg to config

* add:impl interface of ReplicateExistingClusterService

* feat:impl replicate existing cluster's resources logic if ExternalImportEnabled is enabled

* test:add Import with ignoreRestart test

* refactor:add interface of ExportService

* add:create mock of ExportService(run go generate)

* Update config/config.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Update export/export.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Update export/export.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Update replicateexistingcluster/replicateexistingcluster.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Update replicateexistingcluster/replicateexistingcluster.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Update server/di/di.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Update server/di/di.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Update replicateexistingcluster/replicateexistingcluster.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Update replicateexistingcluster/replicateexistingcluster.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Update replicateexistingcluster/replicateexistingcluster.go

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* refactor:rename IgnoreRestart option to IgnoreSchedulerConfiguration

* refactor:rename _exportService to exportService

* refactor:rename to replicateExistingClusterService

* refactor:change convert logic of resources

* refactor:move private functions to utils.go of export package

* test:export utils test

* test:add test for replicateexistingcluster

* fix:return value of ConvertToResourcesForImportFromResourcesForExport

* fix:get a context in arg of ImportFromExistingCluster

* fix:remove comment

* fix:rename to ConvertResourcesForImportToResourcesForExport

* fix:do import before API server starting

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* support reset feature on web ui (#114)

* fix nil pointer reference

* update delete collection method to force delete

* fix conflict

* delete debug print

* fix lint

* fix

* update factory method

* add provider

* fix reset button

* fix format

* Rename simulatorPlugin to wrappedPlugin (#126)

* refactor:rename simulatorPlugin to wrappedPlugin

* fix:prefix word

* test:fix

* refactor:receiver name

* Implement table view on Web UI (#119)

* refactor:move resource view components to under the ResourceViews/List/ directory

* refactor:create ResourcesViewPanel component and replace it on index.vue

* add: ResourcesViewPanel & RadioButton to select view

* add:DataTable template

* fix:typo

* add:basic data tables for each resources

* fix:import PodList on UnscheduledPodList

* fix:change display columns

* fix:declare of Array type

* fix:remove debug comment

* fix:change show columns

* fix:change columns name

* typo:lang

* style:adjust search text field of tables

* feat:add ALPHA_TABLE_VIEWS env

* fix:show datatable if ALPHA_TABLE_VIEWS env val is "1"

* fix:column name

* refactor:separated into new component

* refactor:change label to title

* style:make it clear that each row on table is clickable

* Display the kind/apiVersion in the scheduler configuration yaml (#129)

* Display the kind/apiVersion in the scheduler configuration yaml

* Add kind/apiVersion to the yaml as you might expect
* For "APPLY" keep the profiles only so that ignored edits
  (e.g. apiVersion) are not displayed next time.
* The editor still allows unsupported/ignored edits, but at least
  it is clear what was set when you revisit.

Signed-off-by: Mark Sturdevant <mark.sturdevant@ibm.com>

* Ensure only profiles get updated in ApplySchedulerConfig

* Also don't yarn lint/format coverage directory

* WIP - remove unecessary shutdown fn

* fix rebase issue

* merge master

* Request kube apiserver directly from frontend (#115)

* Request kube apiserver directly from frontend

* Show axios errors on page

* Add axios error judge

* Change default kubeAPIserver port

* Add simple cause err message, Fix typo

* Delete comments

* Fix err message

* change kube apiserver request header

* tmp

* Delete pods when delete node

* delete managed field before apply

* Update web/components/ResourceBar/ResourceBar.vue

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Add deprecate description (#127)

* Add deprecate comment lines

* change some docs description

* fix indent and move not-deprecate APIs

* fix linter err

* setup annoymoususer for insecure testing

* Request kube apiserver directly from frontend (#115)

* Request kube apiserver directly from frontend

* Show axios errors on page

* Add axios error judge

* Change default kubeAPIserver port

* Add simple cause err message, Fix typo

* Delete comments

* Fix err message

* change kube apiserver request header

* tmp

* Delete pods when delete node

* delete managed field before apply

* Update web/components/ResourceBar/ResourceBar.vue

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Able to configure API server and port, thus expose it is possible via compose

* refactor due to linters

* remove unecessary error channel

* log the error return from prepared aggregator server

* add temp cert directory

* remote unnecessary changes.

* update md to reflect default api behavior

* remove unnecessary flag

* Enable Priority Plugin for admission

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* Change the order of logging to ensure url is correct.

Co-authored-by: Kensei Nakada <handbomusic@gmail.com>

* pass cleanupFunc all the way to shutdown

* attemp to fix useragent issue

* Revert "attemp to fix useragent issue"

This reverts commit b06408a.

Co-authored-by: 196Ikuchil <22634362+196Ikuchil@users.noreply.github.com>
Co-authored-by: Kensei Nakada <handbomusic@gmail.com>
Co-authored-by: sivchari <shibuuuu5@gmail.com>
Co-authored-by: Mark Sturdevant <mark.sturdevant@ibm.com>
Co-authored-by: momom-i <51011095+momom-i@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

change frontend to request kube-apiserver directly to CRUD resources and deprecate all APIs for CRUD resources
3 participants