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.

Sign up for GitHub

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

Jump to bottom

Add AKS node pool new options #1122

Merged
merged 1 commit into from May 18, 2023
Merged

Add AKS node pool new options #1122

merged 1 commit into from May 18, 2023

Conversation

mjura
Copy link
Contributor

@mjura mjura commented May 15, 2023

Add labels, taints and max surge options to Node Pool

Fix: #1115

@mjura mjura mentioned this pull request May 15, 2023
Closed
richardcase
richardcase approved these changes May 16, 2023
View reviewed changes
Copy link

@richardcase richardcase left a comment

Choose a reason for hiding this comment

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

LGTM

furkatgofurov7
furkatgofurov7 approved these changes May 16, 2023
View reviewed changes
Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment •
edited

Choose a reason for hiding this comment

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

LGTM

Maybe it is worth mentioning why the go mod changes are needed (TL;DR: the commit hash used for rancher/pkg/apis and rancher/pkg/client were behind and modules update to latest commit hash was needed to make new fields available in the schema) and link the PR which added these new fields to the schema for easier review/understanding

a-blender
a-blender suggested changes May 16, 2023
View reviewed changes
Copy link
Contributor

@a-blender a-blender left a comment

Choose a reason for hiding this comment

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

@mjura Looks good, but please

  1. Add new fields and data types to the docs
  2. Make sure to fill out the PR template, and add steps for how you did manual / automated testing to make sure this works in aks.

@mjura
Add AKS node pool new options
42561f8 
Add labels, taints and max surge options to Node Pool

Fix: rancher#1115
@mjura
Copy link
Contributor Author

mjura commented May 17, 2023

@mjura Looks good, but please

  1. Add new fields and data types to the docs

I have added options to documentation

  1. Make sure to fill out the PR template, and add steps for how you did manual / automated testing to make sure this works in aks.

Example was added and tested with AKS

It is ready to merge

@mjura mjura requested a review from a-blender May 17, 2023 08:04
a-blender
a-blender approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@a-blender a-blender left a comment •
edited

Choose a reason for hiding this comment

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

Looks great @mjura! Go ahead and merge once CI passes.

@mjura mjura merged commit e8bc1e6 into rancher:master May 18, 2023
1 check passed
@mjura mjura mentioned this pull request Jun 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardcase richardcase richardcase approved these changes

@a-blender a-blender a-blender approved these changes

@furkatgofurov7 furkatgofurov7 furkatgofurov7 approved these changes

@rmweir rmweir Awaiting requested review from rmweir

@HarrisonWAffel HarrisonWAffel Awaiting requested review from HarrisonWAffel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@mjura @richardcase @a-blender @furkatgofurov7