Conversation
matthchr
force-pushed
the
matthchr/byo-machine-e2e-base
branch
from
9fccbaf to
3d55777
Compare
matthchr
force-pushed
the
matthchr/machine-tests
branch
from
04513e4 to
d397f58
Compare
matthchr
force-pushed
the
matthchr/byo-machine-e2e-base
branch
from
3d55777 to
636cd61
Compare
tallaxes
left a comment
tallaxes
left a comment
There was a problem hiding this comment.
LGTM, minor suggestions
| interfacesClient *armnetwork.InterfacesClient | ||
| managedClusterClient *containerservice.ManagedClustersClient | ||
| agentpoolsClient *containerservice.AgentPoolsClient | ||
| agentPoolClient *containerservice.AgentPoolsClient |
There was a problem hiding this comment.
Are you driving towards singular in client var names? We are not using singular/plural consistently; all types are plural. Our var names are a mix of singular (vm,vnet,subnet,managedCluster - and now agentPool) and plural (interfaces,machines)
There was a problem hiding this comment.
I was making it more consistent with what we had, but I'm happy to go the other way too, I don't really care too much. Mostly I noticed this because the casing was wrong and I was fixing that.
I could fix either direction further in a separate PR?
| --enable-oidc-issuer --enable-workload-identity --nodepool-taints "CriticalAddonsOnly=true:NoSchedule" \ | ||
| $(if $(AZURE_VM_SIZE),--node-vm-size $(AZURE_VM_SIZE)) \ |
There was a problem hiding this comment.
I presume we have toleration somewhere that still makes Karpenter itself run fine? (TBH, the combination of initial tainting + tainting and untaining in makefile targets is starting to be a little confusing. If we have these initial taints, and Karpenter deployment tolerates them, maybe we don't need to taint/untaint dynamically anymore?)
There was a problem hiding this comment.
This is a good callout... also only in this branch though, will investigate further (tests work but I am not sure how now that you point it out)
There was a problem hiding this comment.
Yes we tolerate this taint:
tolerations:
- key: CriticalAddonsOnly
operator: Exists
636cd61 to
f9c953e
Compare
matthchr
force-pushed
the
matthchr/machine-tests
branch
from
d397f58 to
041ae06
Compare
3 participants
Description
Add machines tests.
This replaces #1212
How was this change tested?
Does this change impact docs?
Release Note