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

Basic cluster reconciler #150

Merged

Conversation

chrigl
Copy link

@chrigl chrigl commented Dec 14, 2018

What this PR does / why we need it:

Implemented basic cluster reconciler. When a Cluster.NodeCIDR and a Cluster.ExternalNetworkID are configured, the cluster reconciler creates an OpenStack Network with an associated Subnet (this will hold the IP range). Additionally an OpenStack router will be create, and attached to the just created Subnet as well as the ExternalNetworkID to get public internet to the VMs. All of the created components are stored in OpenstackClusterProviderStatus.

With this, we are able to change the machine actuator to use the Network from Cluster.Status.ProviderSpec.Network. It could be still possible to configure networks on Machine bases.

This code does not interfere with current cluster creation. If there is no network configuration on the cluster, the cluster actuator only is a noop as before.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #106

Special notes for your reviewer:

In order to test use this image: chrigl/openstack-cluster-api-controller:latest

Release note:


@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 Dec 14, 2018
@chrigl chrigl changed the title Basic cluster reconciler WIP Basic cluster reconciler Dec 21, 2018
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 21, 2018
@chrigl
Copy link
Author

chrigl commented Dec 21, 2018

Needs a rebase and minor change when #136 is in.

@scruplelesswizard
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chaosaffe, chrigl

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 Dec 21, 2018
Christoph Glaubitz added 2 commits December 21, 2018 15:07
Added ports for subnets to router

Implemented cluster update

Replaced glog by klog

Changed usage of NetworService

Now Reconcile creates an instance of NetworkService on each run. So in
theory it will be possible to configure OS-Credentials per cluster.

Added router to ClusterStatus

Changed to only one Router

Resynced Gopkg.lock

Improved logs

Reconciling of networks now log the default log level

Added Docs to ClusterStatus.Network

Recreated zz_generated with new types
@chrigl chrigl changed the title WIP Basic cluster reconciler Basic cluster reconciler Dec 21, 2018
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 21, 2018
@chrigl
Copy link
Author

chrigl commented Dec 21, 2018

Huge rebase necessary because of 136. But everything works now.

Christoph Glaubitz added 3 commits January 3, 2019 15:33
At the current point in time, we don't need multiple subnets, and we are
not even able to configure them.

Beside this, I refactored the reconciling code to return network, subnet
and router instead of manipulating pointer data.
@ainmosni
Copy link
Contributor

ainmosni commented Jan 4, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 4, 2019
@k8s-ci-robot k8s-ci-robot merged commit f1dae38 into kubernetes-sigs:master Jan 4, 2019
flaper87 pushed a commit to flaper87/cluster-api-provider-openstack that referenced this pull request Jan 9, 2019
* Basic implementation of creating networks

Added ports for subnets to router

Implemented cluster update

Replaced glog by klog

Changed usage of NetworService

Now Reconcile creates an instance of NetworkService on each run. So in
theory it will be possible to configure OS-Credentials per cluster.

Added router to ClusterStatus

Changed to only one Router

Resynced Gopkg.lock

Improved logs

Reconciling of networks now log the default log level

Added Docs to ClusterStatus.Network

Recreated zz_generated with new types

* Migrated to params of PR 136

* removed unneeded line

* Updated Network to only hold a single subnet

At the current point in time, we don't need multiple subnets, and we are
not even able to configure them.

Beside this, I refactored the reconciling code to return network, subnet
and router instead of manipulating pointer data.

* Removed unneeded clusterclient in favor of runtime.Client

See also
kubernetes-retired/kubefed#208 (comment)

We don't need the clientset_generated any more.

* Fixed typos, re-ordered fields
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. 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.

Implement basic cluster actuator
5 participants