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

Update to Kubernetes 1.24.12 + Use Go 1.19 #44

Merged
merged 5 commits into from
Apr 14, 2023

Conversation

hrak
Copy link
Contributor

@hrak hrak commented Mar 1, 2023

This PR is a continuation of the work in #32 and contains the following changes:

  • Use Go 1.19
  • Update Kubernetes deps to 1.26.2 1.24.12
  • Move the cloudstack-go dep to its new home in the apache org (and update to v2.15.0)
  • switch to klog v2

I can provide a test container image if needed.

bump Kubernetes Version to 1.20.6
@hrak hrak changed the title Update to Kubernetes 1.26.2 + Use Go 1.18 Update to Kubernetes 1.26.2 + Use Go 1.19 Mar 1, 2023
@hrak
Copy link
Contributor Author

hrak commented Mar 1, 2023

Please let me know if 1.26 i a bit too ambitious, i will gladly adapt for 1.24 or 1.25.

@joschi36
Copy link
Contributor

joschi36 commented Mar 1, 2023

If you have the time I would really appreciate if there is a version in between. For instance 1.24.

@hrak
Copy link
Contributor Author

hrak commented Mar 1, 2023

If you have the time I would really appreciate if there is a version in between. For instance 1.24.

yeah i was afraid of that already, i suddenly realized that compiling this against 1.26 probably means losing compatibility with 1.24 and 1.25 which are still supported. I will get on it!

@hrak hrak changed the title Update to Kubernetes 1.26.2 + Use Go 1.19 Update to Kubernetes 1.24.11 + Use Go 1.19 Mar 1, 2023
@hrak
Copy link
Contributor Author

hrak commented Mar 1, 2023

Updated PR to use Kubernetes v1.24.11 instead

@hrak
Copy link
Contributor Author

hrak commented Apr 3, 2023

Ship it? Or does anything else need to happen?

Signed-off-by: Hans Rakers <h.rakers@global.leaseweb.com>
Signed-off-by: Hans Rakers <h.rakers@global.leaseweb.com>
Signed-off-by: Hans Rakers <h.rakers@global.leaseweb.com>
@hrak
Copy link
Contributor Author

hrak commented Apr 12, 2023

Added a switch to klog v2 + updated PR for k8s 1.24.12

@hrak hrak changed the title Update to Kubernetes 1.24.11 + Use Go 1.19 Update to Kubernetes 1.24.12 + Use Go 1.19 Apr 12, 2023
Signed-off-by: Hans Rakers <h.rakers@global.leaseweb.com>
@RolphH
Copy link

RolphH commented Apr 13, 2023

@rohityadavcloud can you help getting this one merged?

Copy link
Member

@rohityadavcloud rohityadavcloud left a comment

Choose a reason for hiding this comment

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

LGTM but I didn’t test it

@rohityadavcloud
Copy link
Member

@davidjumani could you have a quick look and help merge it.

@RolphH @hrak could you advise what testing you’ve done against any/various versions of CloudStack?

@hrak
Copy link
Contributor Author

hrak commented Apr 13, 2023

@davidjumani could you have a quick look and help merge it.

@RolphH @hrak could you advise what testing you’ve done against any/various versions of CloudStack?

We are running this without any issues so far on Cloudstack 4.17. We have some more changes in the pipeline once this is merged.

@hrak
Copy link
Contributor Author

hrak commented Apr 14, 2023

We are running this without any issues so far on Cloudstack 4.17. We have some more changes in the pipeline once this is merged.

Well, maybe i should clarify: The part that works is the loadbalancer/firewall rules part. Most of the logic surrounding instance metadata is broken ever since this was turned into and external cloud provider. GetZone assumes this is running inside kubelet and that hostname=instance hostname, but it isn't, since the provider runs in a pod, causing it to try to look up the instance based on the pod hostname:

I0414 07:08:41.827320       1 cloudstack.go:175] GetZone called from /go/src/github.com/apache/cloudstack-kubernetes-provider/cloudstack_instances.go#198
E0414 07:08:41.874064       1 node_controller.go:220] error syncing 'k8s-mgmt-cp-dev-02': failed to get instance metadata for node k8s-mgmt-cp-dev-02: could not find instance for retrieving the zone: No match found for cloud-controller-manager-789b9fb546-dg46l: &{Count:0 VirtualMachines:[]}, requeuing

Then there are issues with ProviderID, since the provider is currently called external-cloudstack and should probably be called cloudstack, so the logic here matches the kubelet arguments --cloud-provider=external --provider-id=cloudstack:///8411c37d-8d63-4920-8cb2-01fa512c3377 in the same way that this works for f.e. the Openstack CCM. Since most cloud controllers will be external soon and there is no more internal cloudstack CCM the external- bit is kind of redundant anyway.

Passing --cloud-provider=external to the kubelet causes it to taint all nodes with node.cloudprovider.kubernetes.io/uninitialized=true:NoSchedule, which gets removed once the cloud provider is initialized properly. But because of above mentioned issues, the taint never gets removed, causing a non-functional cluster.

I am working on fixes for all these issues but i would really like to get this in first.

@rohityadavcloud
Copy link
Member

Thanks @RolphH @hrak I'll merge this based on your tests and feedback. Pl do followup with other changes you think we need for the codebase.

@rohityadavcloud rohityadavcloud merged commit c939508 into apache:main Apr 14, 2023
1 check passed
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

6 participants