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

Add CVM v6 SKUs #3474

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Add CVM v6 SKUs #3474

merged 1 commit into from
Oct 23, 2024

Conversation

kamalca
Copy link
Collaborator

@kamalca kamalca commented Oct 14, 2024

CVM v6 SKUs need to be added to the list of CVM supporting SKUs.

@kamalca
Copy link
Collaborator Author

kamalca commented Oct 14, 2024

@squirrelsc @LiliDeng I don't like this method of hard coding SKU families. I don't understand the original justification for it. Is there a better solution?

@kamalca
Copy link
Collaborator Author

kamalca commented Oct 14, 2024

I see this commit 313cefd is where the check for CVM SKU families is added, but #2370 provides no justification for why we need to check this. We should trust the capability tagging on the SKUs and we should avoid creating more work for ourselves needing to manually update SKU family lists.

@LiliDeng
Copy link
Collaborator

I see this commit 313cefd is where the check for CVM SKU families is added, but #2370 provides no justification for why we need to check this. We should trust the capability tagging on the SKUs and we should avoid creating more work for ourselves needing to manually update SKU family lists.

Do you know the Capabilities names for CVM and Stateless? if yes, we can update coed to reply on them.

@kamalca kamalca force-pushed the kameroncarr/cvm-deployment branch from 31fa7f8 to c099ce0 Compare October 21, 2024 21:27
@kamalca kamalca force-pushed the kameroncarr/cvm-deployment branch from c099ce0 to 436a25b Compare October 21, 2024 22:03
CVM v6 SKUs are being added. We should reduce the need to update lists of SKU families by relying on the capabilities.
@kamalca kamalca force-pushed the kameroncarr/cvm-deployment branch from 436a25b to f0750ea Compare October 21, 2024 22:18
@squirrelsc
Copy link
Member

@LiliDeng LGTM

@LiliDeng
Copy link
Collaborator

LiliDeng commented Oct 22, 2024

@kamalca could you please make sure you have tested this change? Thanks.

@kamalca
Copy link
Collaborator Author

kamalca commented Oct 23, 2024

@kamalca could you please make sure you have tested this change? Thanks.

I tested SNP and TDX deployment. I am currently running all of my CVM v6 validation with this change and have seen no issues.

@kamalca kamalca merged commit 40e352b into main Oct 23, 2024
45 checks passed
@kamalca kamalca deleted the kameroncarr/cvm-deployment branch October 23, 2024 17: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.

3 participants