Skip to content

Pr v1alpha2 further refactorings#455

Merged
k8s-ci-robot merged 10 commits intokubernetes-sigs:masterfrom
sbueringer:pr-misc-refactorings-v1alpha2
Sep 2, 2019
Merged

Pr v1alpha2 further refactorings#455
k8s-ci-robot merged 10 commits intokubernetes-sigs:masterfrom
sbueringer:pr-misc-refactorings-v1alpha2

Conversation

@sbueringer
Copy link
Member

@sbueringer sbueringer commented Sep 1, 2019 •
edited
Loading

What this PR does / why we need it:
More refactorings for v1alpha2

This includes: (mostly syncs with CAPA)

  • Logging (removed klog)/ event cleanup
  • Makefile
  • Switched to golangci linting (& fixed a lot of lint findings)
  • Synced config folder
  • Renamed keyname to sshKeyName
  • Moved tools.go to hack/tools/
  • Added some flags to the controller (like cluster & machine concurrency)
  • Implement MachineDeployments

I updated the list of open issues for v1alpha2 here: #380 (comment)

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 1, 2019
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 1, 2019
@sbueringer sbueringer mentioned this pull request Sep 1, 2019
Closed
@sbueringer sbueringer changed the title [WIP] Pr v1alpha2 further refactorings Pr v1alpha2 further refactorings Sep 1, 2019
@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 Sep 1, 2019
@sbueringer
Copy link
Member Author

sbueringer commented Sep 1, 2019

/assign @hidekazuna
/assign @jichenjc

hidekazuna
hidekazuna reviewed Sep 2, 2019
View reviewed changes
config/default/manager_image_patch.yaml Outdated
containers:
# Change the value of image field below to your controller image URL
- image: docker.io/sbueringer/cluster-api-provider-openstack:dev
- image: k8scloudprovider/cluster-api-openstack-controller-amd64:dev
Copy link
Contributor

@hidekazuna hidekazuna Sep 2, 2019

Choose a reason for hiding this comment

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

Image name should be cluster-api-openstack-controller:latest if we refer to aws.
https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/master/config/default/manager_image_patch.yaml#L11

Copy link
Member Author

@sbueringer sbueringer Sep 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I would use the one published to dockerhub for now:
https://hub.docker.com/r/k8scloudprovider/openstack-cluster-api-controller/tags

(no idea how it's published to docker hub)

Let's adjust it when we implement the image release process

Copy link
Contributor

@hidekazuna hidekazuna Sep 2, 2019

Choose a reason for hiding this comment

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

The job in OpenLab publishes the images periodically.
https://github.com/theopenlab/openlab-zuul-jobs/blob/master/zuul.d/projects.yaml#L205

I agree to adjust when we discuss release process.

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 2, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

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

The pull request process is described here

Details 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 Sep 2, 2019
@hidekazuna
Copy link
Contributor

hidekazuna commented Sep 2, 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 Sep 2, 2019
@k8s-ci-robot k8s-ci-robot merged commit 7e4e4a8 into kubernetes-sigs:master Sep 2, 2019
@sbueringer sbueringer deleted the pr-misc-refactorings-v1alpha2 branch October 6, 2019 12:08
@EmilienM EmilienM mentioned this pull request Jan 31, 2023
Closed
pierreprinetti pushed a commit to shiftstack/cluster-api-provider-openstack that referenced this pull request Apr 22, 2024
@sbueringer @pierreprinetti
Pr v1alpha2 further refactorings (kubernetes-sigs#455)
532f1c5 
* further refactorings for v1alpha2

* test fix & define targets for prow

* regen & fix typo in log message

* fix tests

* fix gen examples

* minor renaming,

* nil pointer fix

* implement machinedeployment

* fix tests

* review fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lion-Wei Lion-Wei Awaiting requested review from Lion-Wei

@vincepri vincepri Awaiting requested review from vincepri

1 more reviewer

@hidekazuna hidekazuna hidekazuna left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@hidekazuna hidekazuna

@jichenjc jichenjc

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sbueringer @k8s-ci-robot @hidekazuna @jichenjc

Comments