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

Upgrade CAPI deps #892

Merged
merged 12 commits into from Mar 22, 2022
Merged

Upgrade CAPI deps #892

merged 12 commits into from Mar 22, 2022

Conversation

fiunchinho
Copy link
Member

@fiunchinho fiunchinho commented Mar 11, 2022

Towards https://github.com/giantswarm/giantswarm/issues/21301

Most of the changes you see on this PR are due to the length in description text fields.

Checklist

  • Consider SIG UX feedback.
  • Update changelog in CHANGELOG.md.

@fiunchinho fiunchinho self-assigned this Mar 11, 2022
@@ -100,7 +100,8 @@ spec:
ssh:
userList:
- name: joe
publicKey: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQCuJvxy3FKGrfJ4XB5exEdKXiqqteXEPFzPtex6dC0lHyigtO7l+NXXbs9Lga2+Ifs0Tza92MRhg/FJ+6za3oULFo7+gDyt86DIkZkMFdnSv9+YxYe+g4zqakSV+bLVf2KP6krUGJb7t4Nb+gGH62AiUx+58Onxn5rvYC0/AXOYhkAiH8PydXTDJDPhSA/qWSWEeCQistpZEDFnaVi0e7uq/k3hWJ+v9Gz0q---SHORTENED---G7iIV0Y6o9w5gIHJxf6+8X70DCuVDx9OLHmjjMyGnd+1c3yTFMUdugtvmeiGW== joe
publicKey: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQCuJvxy3FKGrfJ4XB5exEdKXiqqteXEPFzPtex6dC0lHyigtO7l+NXXbs9Lga2+Ifs0Tza92MRhg/FJ+6za3oULFo7+gDyt86DIkZkMFdnSv9+YxYe+g4zqakSV+bLVf2KP6krUGJb7t4Nb+gGH62AiUx+58Onxn5rvYC0/AXOYhkAiH8PydXTDJDPhSA/qWSWEeCQistpZEDFnaVi0e7uq/k3hWJ+v9Gz0q---SHORTENED---G7iIV0Y6o9w5gIHJxf6+8X70DCuVDx9OLHmjjMyGnd+1c3yTFMUdugtvmeiGW==
joe
Copy link
Contributor

Choose a reason for hiding this comment

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

@fiunchinho how the heck this change happened? 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

I wish I knew

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it's this bug kubernetes-sigs/yaml#68

@fiunchinho fiunchinho marked this pull request as ready for review March 17, 2022 15:41
@fiunchinho fiunchinho requested a review from a team March 17, 2022 15:41
Copy link
Contributor

@MarcelMue MarcelMue left a comment

Choose a reason for hiding this comment

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

OK from my PoV but phoenix knows best.

@@ -164,7 +164,9 @@ spec:
could not be found in the time allotted, and the client
should retry (optionally after the time indicated in the
Retry-After header). \n Applied only if Name is not specified.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#idempotency"
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#idempotency
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we still have these upstream CRDs? Aren't they managed by the CAPA/CAPZ apps now?

Copy link
Member Author

Choose a reason for hiding this comment

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

The experimental CRDs for MachinePool and AzureMachinePool got moved to a new api group, but we are using them since they were on the old group. This means that if we upgrade, these types are gone, so our operators wouldn't be able to reconcile them. We want to ensure the CRDs and we want to go types available for our operators hence we have them here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess azure-operator should have a crd-install hook that installs these then. But fine for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

After upgrading azure-operator CAPI dependencies (to be done after this PR is merged) and migrating customer MachinePools / AzureMachinePools we can get rid of these experimental CRDs, so we won't invest on doing the hook.

@fiunchinho fiunchinho merged commit 6c7f895 into master Mar 22, 2022
@fiunchinho fiunchinho deleted the update-capi-deps branch March 22, 2022 18:07
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

4 participants