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 metrics-server-operator #73

Merged

Conversation

atoato88
Copy link
Contributor

@atoato88 atoato88 commented Feb 5, 2020

This PR adds metrics-server-operator to examples/.
As of now dashboard-operator is located in examples/.
Dashboard v2.0.0 which is next release can use metrics-server for
monitoring statistics of k8s nodes.

It's better to add metrics-server-operator to this repo because that
dashboard v2.0.0 will release before long.

When dashboard v2.0.0 has released, it is needed to add v2.0.0
manifests to dashboard-operator.

This commit adds metrics-server-operator to `examples/`.
As of now dashboard-operator is located in `examples/`.
Dashboard v2.0.0 which is next relase can use metrics-server for
monitoring statistics of k8s nodes.

It's better to add metrics-server-operator to this repo because that
dashboard v2.0.0 will release before long.

When dashboard v2.0.0 has released, it is needed to add v2.0.0
manifests to dashboard-operator.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 5, 2020
@k8s-ci-robot k8s-ci-robot requested review from droot and mengqiy February 5, 2020 02:15
@k8s-ci-robot k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Feb 5, 2020
@atoato88
Copy link
Contributor Author

/assign @johnsonj
Could you review this?

Copy link
Contributor

@johnsonj johnsonj left a comment

Choose a reason for hiding this comment

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

Something to consider: This could also live in kubernetes-sigs/cluster-addons. It may also make sense to duplicate the dashboard-operator there?

Overall looks good, please address the super-minor assign copyright comment.

@@ -0,0 +1,36 @@
/*
Copyright 2020 TODO(akihito): assign copyright.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think The Kubernetes Authors is standard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for comment. I've fixed these.

@johnsonj
Copy link
Contributor

Thank you @atoato88 !

@johnsonj
Copy link
Contributor

/lgtm
/approved

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 21, 2020
@johnsonj
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atoato88, johnsonj

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 Feb 21, 2020
@k8s-ci-robot k8s-ci-robot merged commit 25b6e29 into kubernetes-sigs:master Feb 21, 2020
@atoato88
Copy link
Contributor Author

@johnsonj
Thank you for approving!!

I also thought what you considered. 😃
dashboard-operator and metrics-server-operator in this repository is examples for this framework.
But the same time, it is reasonable to place in kubernetes-sigs/cluster-addons because of it's truely addons. 😃

As you comment, we can move these to kubernetes-sigs/cluster-addons.
But I think that it should not be duplicated to two places(kubebuilder-declarative-patterns/examples/ and cluster-addons/) becuase of avoiding double management.
WDYT?

Should we use git submodule?

  1. move codes(relating to dashboard-operator and metrics-server-operator) in kubebuilder-declarative-patterns/examples to cluster-addons repository
  2. remove directory examples on kubebuilder-declarative-patterns
  3. on kubebuilder-declarative-patterns, git submodule add https://github.com/kubernetes-sigs/cluster-addons.git examples

But this may need some code changes import in *.go files and go.mod.

Or, is git submodule something tricky?

@johnsonj
Copy link
Contributor

I think you nailed it that dashboard is an addon and duplication is bad news. Submodules are tough to manage. If we want to make the dashboard-operator useful (and not just an illustrative example of how to use this library) then I think it just doesn't belong here.

What about a wilder turn:

  1. Move {dashboard, metrics-server}-operator to cluster-addons
  2. Replace dashbboard-operator with something where we don't care about versions, like guestbook-operator (as in this guestbook or the go version). We can just use it to show 'how it's made'

@atoato88
Copy link
Contributor Author

I agree with your idea. 👍
It's a reasonable suggestion.

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants