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

feat: v1 migration and adaptation #528

Merged
merged 50 commits into from
Oct 24, 2024
Merged

feat: v1 migration and adaptation #528

merged 50 commits into from
Oct 24, 2024

Conversation

tallaxes
Copy link
Collaborator

@tallaxes tallaxes commented Oct 16, 2024

Description

Update to Karpenter v1 API, with many follow-ups. Summary of changes (mostly grouped by commits):

  • Karpenter core update: update to v1.0.4, refresh and relink core CRDs, update main and alt operator; migrate cloud provider, operator, providers (convert to interfaces), controllers, test package and E2E tests to v1 API
  • AKSNodeClass: introduce kubelet config; add nodeclass status, termination and hash controllers
  • Development: bump Go version to 1.23, bump dependencies, refresh toolchain, fix linting (golang-ci was silently failing; move codegen into subfolders), update lint configuration (enable most of govet, exclude alt operator logging), restore linting on verify, refresh pre-commit components
  • Deployment: deploy to kube-system by default, update Helm charts (many changes), update karpenter-values-template (remove drift feature gate); enable webhooks (including webhook injection into CRDs); includes enablement of webhooks in CCP context (thank you, @matthchr!)
  • Testing: improve selection of karpenter pods for debug logs on failure, add events and simplify; fix utilization suite; add cilium label and taint; fix labels and disruption for daemonset test

How was this change tested?

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note


Next

AKSNodeClass

  • Propagate kubelet config (and add tests)
  • Add more CEL constraints
  • Add CEL tests
  • Review conditions (including properly computing readiness)

Testing

Other

  • Review instance provider CreateTags
  • NodeClaim tagging controller

(and migration to kube-system)
@tallaxes tallaxes merged commit 46fc0d3 into main Oct 24, 2024
20 of 21 checks passed
@tallaxes tallaxes deleted the tallaxes/v1-migration branch October 24, 2024 22:54
Data: map[string][]byte{
certresources.ServerKey: []byte("present"),
certresources.ServerCert: []byte("present"),
// certresources.CACert: []byte("missing"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

commented, can be removed?

)

//go:generate controller-gen crd object:headerFile="../../hack/boilerplate.go.txt" paths="./..." output:crd:artifacts:config=crds
var (
Group = "karpenter.azure.com"
//CompatibilityGroup = "compatibility." + Group
Copy link
Collaborator

Choose a reason for hiding this comment

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

commented out, can be removed?

This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it should be camelCase, should we enforce the first letter is lowercase?

enum:
- "True"
- "False"
- Unknown
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should Unknown also be quoted?

This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we enforce starting with a lowercase letter if it should be camel case?

enum:
- "True"
- "False"
- Unknown
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should Unknown be quoted?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Issues or PRs related to the APIs area/dependency Issues or PRs related to dependency changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants