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

user-cluster tool #2350

Closed
wants to merge 3 commits into from
Closed

user-cluster tool #2350

wants to merge 3 commits into from

Conversation

kron4eg
Copy link
Member

@kron4eg kron4eg commented Nov 11, 2018

What this PR does / why we need it:
Syncier asked for CI tool to automate creation of user-clusters

user-cluster tool, to automate user-cluster creation

labels:
user: << Optional UserID to see cluster in UI>>
worker-name: ""
spec:
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not look right

Copy link
Member Author

Choose a reason for hiding this comment

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

please elaborate what do you mean


### Example AWS Node spec

spec:
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please add the rest of the object?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's complete object (used by kubermatic http api) github.com/kubermatic/kubermatic/api/pkg/api/v1.Node


### Example AWS Cluster spec

apiVersion: kubermatic.k8s.io/v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please highlight both code blocks as yaml?

kubermatic/kubermatic user-cluster

## Flags
TBD
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document the current flags or remove this section

# Automate creation of kubermatic managed clusters

## What is this
It's a tool to automate creating of custom resources for kubermatic managed
Copy link
Contributor

Choose a reason for hiding this comment

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

remove line break

Copy link
Member Author

Choose a reason for hiding this comment

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

why? it's 80-th column text-wrap

)

const (
generateName = "auto-cluster"
Copy link
Contributor

Choose a reason for hiding this comment

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

the user should be able to configure this by himself (new flag?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see a reason why. That tool is for CI. It creates those "automatic user clusters".

}

for i := 0; i < ctl.runOpts.Nodes; i++ {
template.Name = fmt.Sprintf("%s%s", generateName, rand.String(5))
Copy link
Contributor

Choose a reason for hiding this comment

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

Use template.GenerateName as a base when generating the machine name

Copy link
Contributor

@alvaroaleman alvaroaleman left a comment

Choose a reason for hiding this comment

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

Why do we put a project-specific tool into the main kubermatic/kubermatic repo?

And why don't we leverage our API?

@kron4eg
Copy link
Member Author

kron4eg commented Nov 12, 2018

@alvaroaleman

Why do we put a project-specific tool into the main kubermatic/kubermatic repo?

First of all, although user-cluster is for syncier, it will be useful for us too (for CI purpose), and for everyone basically. Second, since kubermatic/kubermatic is a monorepo where else should we put this tool?

And why don't we leverage our API?

That'd mean to start whole new project, generating api-client out of swagger spec, wrapping everything in nice CLI, dealing with (non-existing yet Service Accounts). That's not an objective for this tool. What we need here is to:

  • create user-cluster control-plain
  • create machines
  • dump user-cluster kubeconfig

@alvaroaleman
Copy link
Contributor

First of all, although user-cluster is for syncier, it will be useful for us too (for CI purpose), and for everyone basically. Second, since kubermatic/kubermatic is a monorepo where else should we put this tool?

We already have the testrunner for that and don't need another tool

That'd mean to start whole new project, generating api-client out of swagger spec, wrapping everything in nice CLI, dealing with (non-existing yet Service Accounts). That's not an objective for this tool. What we need here is to:

Is there a time constraint on this? UI folks apparently already found a way to get tokens for usage in scripts in their testing. It should be pretty easy after that

@kron4eg
Copy link
Member Author

kron4eg commented Nov 12, 2018

We already have the testrunner

Please point me to one

Is there a time constraint on this?

As always, like, previous week :)

@alvaroaleman
Copy link
Contributor

Please point me to one

https://github.com/kubermatic/kubermatic/tree/c5853e722f9b47fca019759021f38470cd7aad7c/api/cmd/conformance-tests

Is there a time constraint on this?

As always, like, previous week :)

Can you still check in with the api team how difficult that would be? I don't think this will take an incredible amount of time

@kron4eg
Copy link
Member Author

kron4eg commented Nov 12, 2018

@alvaroaleman those are incomparable. conformance-tests is for running "conformance-tests" statically compiled into it...
user-cluster is general purpose tool that will only prepare user-cluster and let users do whatever they want to do with a cluster.

@alvaroaleman
Copy link
Contributor

@alvaroaleman those are incomparable. conformance-tests is for running "conformance-tests" statically compiled into it...

Yes I know they are not the same but for our tests which you stated an example on which we ourselves need the code you add here we do not need this code because we already have the conformance-tests cli

user-cluster is general purpose tool that will only prepare user-cluster and let users do whatever they want to do with a cluster.

We specifically do not want a general-purpose tool that does not use the api

@kron4eg
Copy link
Member Author

kron4eg commented Nov 12, 2018

@alvaroaleman I'm not sure I follow where this leads

alvaroaleman requested changes an hour ago

so which changes are you requesting exactly?

@alvaroaleman
Copy link
Contributor

so which changes are you requesting exactly?

This tool to use our API instead of directly working on CRDs

@kron4eg
Copy link
Member Author

kron4eg commented Nov 12, 2018

what's the problem to work directly on CRDs?

@alvaroaleman
Copy link
Contributor

what's the problem to work directly on CRDs?

The crds are an implementation detail of ours and not an API we want to expose. We want and do change them without notifying external parties

@bashofmann
Copy link
Contributor

The crds are an implementation detail of ours and not an API we want to expose. We want and do change them without notifying external parties

I would argue that CRDs are a public API and should not be changed without notifying external parties, and if they are changed backwards compatibility should be kept in mind and CRDs should be versioned correctly.

@alvaroaleman
Copy link
Contributor

I would argue that CRDs are a public API and should not be changed without notifying external parties, and if they are changed backwards compatibility should be kept in mind and CRDs should be versioned correctly.

No, neither kubermatic cluster crd nor the machine crd are public apis. The first one is only used internally and should not be used externally, the second one is declared alpha by upstream which means they do incompatible changes to it and the pace at which we do or do not adapt upstream changes is a call we make.

CRD versioning is currently not possible, it got merged two days ago into kubernetes master and will maybe land in 1.13. Right now it is possible to specify more than one version but all of them must be identical because there is no conversion.
Even if it was possible with 1.13 we won't use it for the cluster-api types, as we support Kube back to 1.10 for user clusters which does not have crd versioning.

We may decide to pick up crd versioning for Kubermatic but even then, the kubermatic crd is not a public api. Our API is the public api.

@kron4eg
Copy link
Member Author

kron4eg commented Nov 12, 2018

Our API is the public api.

And we don't have anything to work with it besides swagger file, right?

@alvaroaleman
Copy link
Contributor

And we don't have anything to work with it besides swagger file, right?

Yes. You can generate a client from the swagger thought, @glower already did that at some point

@toschneck
Copy link
Member

@kron4eg, @alvaroaleman my thoughts on this:

  • For the most people CRDs are seen as public APIs. If this is correct (regarding to versioning issues) or not is another thing, but if people will see CRDs on the cluster, they will use it. Also popular frameworks like Prometheus use CRDs as abstraction layer for there API, coreos/prometheus-operator#customresourcedefinitions. We also have the machine CRDs in the user cluster whats part of the ClusterAPI, so for me this an abstraction layer for the real API of the operator.

  • The purpose of the tool is, that we run CI/CD load of customer in automated provisioned "ad-hoc clusters". So yeah it would be better to keep it outside of main project and just import the types. But it don't know how you guys manage your dependencies. In my opinion this tool need to be simple and flexible. The basic idea was following use case:

  1. CI job trigger the user-cluster-tool
  2. tool create a cluster based on some cluster.yaml CRD template
  3. extract the kubeconfig for the new created cluster
  4. run the CI/CD stuff with the applied kubeconfig
  5. CI job call tool again and cleanup the cluster.

To reach this, using the CRDs is for me the easiest and most flexible way. If the CRDs are not part of an API the whole clusterAPI project sounds weird for me.

@toschneck toschneck changed the title Syncier user-cluster tool user-cluster tool Nov 13, 2018
@kron4eg kron4eg closed this Nov 16, 2018
@kron4eg kron4eg deleted the syncier-user-clusters-tool branch November 16, 2018 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants