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

Remove non needed 2nd kubernetes node #56

Merged
merged 4 commits into from
Oct 25, 2023
Merged

Conversation

cmoulliard
Copy link
Contributor

Signed-off-by: cmoulliard <[email protected]>
@cmoulliard cmoulliard marked this pull request as ready for review October 23, 2023 12:18
@nabuskey
Copy link
Collaborator

I like the ideas and the cluster itself needs to be configurable. Instead of exposing two specific configuration options, can we let users define the kind cluster config instead? So the flag would be something like --kind-config /path/to/config

The config itself is just the kind cluster config e.g.

kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
networking:
  ipFamily: ipv6

I think we should have the default config embedded in Go like we are doing now, but allow for customization.

@cmoulliard
Copy link
Contributor Author

cmoulliard commented Oct 24, 2023

Instead of exposing two specific configuration options, can we let users define the kind cluster config instead? So the flag would be something like --kind-config /path/to/config

I think that we still need to have some parameters like: name, kube version, extraPorts and perhaps too the server IP address to be used or container type: docker, podman.

I will include the option to pass as additional parameter a kind config file.

Warning: Using such a config file could break what idpbuilder is doing internally. Example, if the config file don't include the proper registry ports, then it will not be possible to pull/push images from the docker registry container. Idem if the ports exposed don't match what ingress will register as routes, etc

Example:
if we pass as name local within this command:
./idpbuilder create --buildName local --kindConfig /path/to/my-kind.cfg
then the hostname of the registry should be equal to here within the kind config

containerdConfigPatches:
- |-
...
    endpoint = ["http://idpbuilder-local-registry:5000"]

@cmoulliard
Copy link
Contributor Author

can we let users define the kind cluster config instead?

See commit: 964bb45 @nabuskey

Copy link
Collaborator

@nabuskey nabuskey left a comment

Choose a reason for hiding this comment

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

Looks good except for a minor documentation line.

What would be cool is to have the ability to dump the default kind configuration file into stdout. This allows end users to easily customize kind clusters without reading too much into documentation. We can place comments in the yaml file about requirements. Out of scope for this PR though. I'll make a new issue.

Thank you for this!

README.md Outdated
`./idpbuilder create --extraPorts 22:32222`

It is also possible to use your own kind config file
`./idpbuilder create --buildName local --kindConfig ./my-kind.cfg`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be ./my-kind.yaml. Kind cluster configs are usually a yaml file I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be ./my-kind.yaml. Kind cluster configs are usually a yaml file I think.

Yes but it can be suffixed with *.cfg too ;-) I will rename it

@nabuskey
Copy link
Collaborator

Opened a new issue related to the comment above. #58

@nabuskey nabuskey merged commit 2923d6c into cnoe-io:main Oct 25, 2023
@nimakaviani nimakaviani added this to the v0.1 release milestone Oct 26, 2023
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.

4 participants