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

Configure provider-id for the machines/nodes #1723

Merged
merged 3 commits into from
Oct 23, 2023

Conversation

ahmedwaleedmalik
Copy link
Member

@ahmedwaleedmalik ahmedwaleedmalik commented Oct 20, 2023

What this PR does / why we need it:
machine-controller will inject provider-id for the machines that are created against cloud providers that don't have in-tree or external CCM support. The generated providerID will be configured on the machine and node respectively.

Which issue(s) this PR fixes:

Fixes #1552

What type of PR is this?

/kind feature

Special notes for your reviewer:

Does this PR introduce a user-facing change? Then add your Release Note here:

Machine-controller will inject provider-id for the machines that are created against cloud providers that don't have in-tree or external CCM support

Documentation:

NONE

MC will inject provider-id for the machines that are created against cloud providers that don't have in-tree or external CCM support

Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik ahmedwaleedmalik self-assigned this Oct 20, 2023
@kubermatic-bot kubermatic-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. docs/tbd Denotes a PR that needs documentation (change) that will be done later. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Oct 20, 2023
@kubermatic-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubermatic-bot kubermatic-bot added dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. sig/virtualization Denotes a PR or issue as being assigned to SIG Virtualization. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 20, 2023
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@ahmedwaleedmalik ahmedwaleedmalik marked this pull request as ready for review October 23, 2023 05:54
@kubermatic-bot kubermatic-bot added docs/none Denotes a PR that doesn't need documentation (changes). and removed docs/tbd Denotes a PR that needs documentation (change) that will be done later. labels Oct 23, 2023
Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
@kubermatic-bot kubermatic-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 23, 2023
@ahmedwaleedmalik ahmedwaleedmalik changed the title WIP: Configure provider-id for the machines/nodes Configure provider-id for the machines/nodes Oct 23, 2023
@kubermatic-bot kubermatic-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 23, 2023
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2023
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 795d405699d3d5fcb3d662195556b4c2c4475c5c

Copy link
Member

@cnvergence cnvergence left a comment

Choose a reason for hiding this comment

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

/approve

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cnvergence, xmudrii

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

@kubermatic-bot kubermatic-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 23, 2023
@ahmedwaleedmalik
Copy link
Member Author

/test pull-machine-controller-e2e-hetzner pull-machine-controller-e2e-kubevirt

@ahmedwaleedmalik
Copy link
Member Author

pull-machine-controller-e2e-vmware-cloud-director will fail and requires #1619 to fix it.
pull-machine-controller-e2e-kubevirt seems to be flaking on different tests.

@ahmedwaleedmalik
Copy link
Member Author

/test pull-machine-controller-e2e-kubevirt

@kubermatic-triage-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs

Review the full test history

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@ahmedwaleedmalik
Copy link
Member Author

/retest

@kubermatic-bot kubermatic-bot merged commit 34c7d12 into kubermatic:main Oct 23, 2023
21 checks passed
@ahmedwaleedmalik ahmedwaleedmalik deleted the providerid branch October 23, 2023 19:32
@ahmedwaleedmalik
Copy link
Member Author

/cherry-pick release/v1.57

@kubermatic-bot
Copy link
Contributor

@ahmedwaleedmalik: #1723 failed to apply on top of branch "release/v1.57":

Applying: Configure provider-id for the machines/nodes
Using index info to reconstruct a base tree...
M	pkg/cloudprovider/provider/digitalocean/provider.go
M	pkg/cloudprovider/provider/equinixmetal/provider.go
M	pkg/cloudprovider/provider/hetzner/provider.go
M	pkg/cloudprovider/provider/kubevirt/provider.go
M	pkg/cloudprovider/provider/linode/provider.go
M	pkg/cloudprovider/provider/nutanix/provider.go
M	pkg/cloudprovider/provider/opennebula/provider.go
M	pkg/cloudprovider/provider/openstack/provider.go
M	pkg/cloudprovider/provider/vmwareclouddirector/provider.go
M	pkg/cloudprovider/provider/vsphere/provider.go
M	pkg/cloudprovider/provider/vultr/provider.go
M	pkg/controller/machine/controller.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/controller/machine/controller.go
Auto-merging pkg/cloudprovider/provider/vultr/provider.go
CONFLICT (content): Merge conflict in pkg/cloudprovider/provider/vultr/provider.go
Auto-merging pkg/cloudprovider/provider/vsphere/provider.go
Auto-merging pkg/cloudprovider/provider/vmwareclouddirector/provider.go
Auto-merging pkg/cloudprovider/provider/openstack/provider.go
Auto-merging pkg/cloudprovider/provider/opennebula/provider.go
Auto-merging pkg/cloudprovider/provider/nutanix/provider.go
Auto-merging pkg/cloudprovider/provider/linode/provider.go
Auto-merging pkg/cloudprovider/provider/kubevirt/provider.go
Auto-merging pkg/cloudprovider/provider/hetzner/provider.go
Auto-merging pkg/cloudprovider/provider/equinixmetal/provider.go
Auto-merging pkg/cloudprovider/provider/digitalocean/provider.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Configure provider-id for the machines/nodes
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release/v1.57

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ahmedwaleedmalik
Copy link
Member Author

@ahmedwaleedmalik: #1723 failed to apply on top of branch "release/v1.57":

Applying: Configure provider-id for the machines/nodes
Using index info to reconstruct a base tree...
M	pkg/cloudprovider/provider/digitalocean/provider.go
M	pkg/cloudprovider/provider/equinixmetal/provider.go
M	pkg/cloudprovider/provider/hetzner/provider.go
M	pkg/cloudprovider/provider/kubevirt/provider.go
M	pkg/cloudprovider/provider/linode/provider.go
M	pkg/cloudprovider/provider/nutanix/provider.go
M	pkg/cloudprovider/provider/opennebula/provider.go
M	pkg/cloudprovider/provider/openstack/provider.go
M	pkg/cloudprovider/provider/vmwareclouddirector/provider.go
M	pkg/cloudprovider/provider/vsphere/provider.go
M	pkg/cloudprovider/provider/vultr/provider.go
M	pkg/controller/machine/controller.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/controller/machine/controller.go
Auto-merging pkg/cloudprovider/provider/vultr/provider.go
CONFLICT (content): Merge conflict in pkg/cloudprovider/provider/vultr/provider.go
Auto-merging pkg/cloudprovider/provider/vsphere/provider.go
Auto-merging pkg/cloudprovider/provider/vmwareclouddirector/provider.go
Auto-merging pkg/cloudprovider/provider/openstack/provider.go
Auto-merging pkg/cloudprovider/provider/opennebula/provider.go
Auto-merging pkg/cloudprovider/provider/nutanix/provider.go
Auto-merging pkg/cloudprovider/provider/linode/provider.go
Auto-merging pkg/cloudprovider/provider/kubevirt/provider.go
Auto-merging pkg/cloudprovider/provider/hetzner/provider.go
Auto-merging pkg/cloudprovider/provider/equinixmetal/provider.go
Auto-merging pkg/cloudprovider/provider/digitalocean/provider.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Configure provider-id for the machines/nodes
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Will do a manual backport

ahmedwaleedmalik added a commit to ahmedwaleedmalik/machine-controller that referenced this pull request Mar 4, 2024
* Configure provider-id for the machines/nodes

MC will inject provider-id for the machines that are created against cloud providers that don't have in-tree or external CCM support

Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>

* Add exception in golangci-lint

Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>

* Refactored code

Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>

---------

Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
kubermatic-bot pushed a commit that referenced this pull request Mar 4, 2024
* Configure provider-id for the machines/nodes

MC will inject provider-id for the machines that are created against cloud providers that don't have in-tree or external CCM support



* Add exception in golangci-lint



* Refactored code



---------

Signed-off-by: Waleed Malik <ahmedwaleedmalik@gmail.com>
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. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. docs/none Denotes a PR that doesn't need documentation (changes). kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-management Denotes a PR or issue as being assigned to SIG Cluster Management. sig/virtualization Denotes a PR or issue as being assigned to SIG Virtualization. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug/feat: cluster-autscaler
5 participants