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 proposal for automating AKS features #4304

Merged
merged 1 commit into from
Feb 15, 2024
Merged

add proposal for automating AKS features #4304

merged 1 commit into from
Feb 15, 2024

Conversation

nojnhuh
Copy link
Contributor

@nojnhuh nojnhuh commented Nov 22, 2023

What type of PR is this?
/kind proposal

What this PR does / why we need it: This PR adds a proposal for implementing #3629.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

  • cherry-pick candidate

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

NONE

/hold for squash

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/proposal Issues or PRs related to proposals. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 22, 2023
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 22, 2023
@nojnhuh
Copy link
Contributor Author

nojnhuh commented Nov 22, 2023

The Summary/Motivation sections should be reasonably complete. Looking for feedback on those sections before digging into the actual proposal.

/cc @CecileRobertMichon @dtzar

@codecov Codecov
Copy link

codecov bot commented Nov 22, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d3332bf) 62.46% compared to head (8daff40) 62.46%.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #4304   +/-   ##
=======================================
  Coverage   62.46%   62.46%           
=======================================
  Files         192      192           
  Lines       15394    15394           
=======================================
  Hits         9616     9616           
  Misses       5118     5118           
  Partials      660      660

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nojnhuh
Copy link
Contributor Author

nojnhuh commented Nov 28, 2023

I think this is reasonably complete and ready for review, though maybe a bit thin in parts.

/retitle add proposal for automating AKS features

@k8s-ci-robot k8s-ci-robot changed the title [WIP] add proposal for automating AKS features add proposal for automating AKS features Nov 28, 2023
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 28, 2023
nawazkh
nawazkh reviewed Dec 1, 2023
View reviewed changes
Copy link
Member

@nawazkh nawazkh left a comment

Choose a reason for hiding this comment

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

Great work putting this together @nojnhuh ! 👏🏼

docs/proposals/20231122-automate-aks-features.md Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Outdated Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Outdated Show resolved Hide resolved
CecileRobertMichon
CecileRobertMichon reviewed Dec 11, 2023
View reviewed changes
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

I'm strongly leaning for option 4

docs/proposals/20231122-automate-aks-features.md Outdated Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Outdated Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Outdated Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Outdated Show resolved Hide resolved
docs/proposals/20231122-automate-aks-features.md Show resolved Hide resolved
@nojnhuh
Copy link
Contributor Author

nojnhuh commented Dec 11, 2023

/cc @matthchr

Interested to see what you might think here.

@k8s-ci-robot
Copy link
Contributor

@nojnhuh: GitHub didn't allow me to request PR reviews from the following users: matthchr.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @matthchr

Interested to see what you might think here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@dtzar dtzar added this to the v1.13 milestone Dec 14, 2023
@mboersma mboersma self-assigned this Dec 14, 2023
@nojnhuh nojnhuh mentioned this pull request Jan 31, 2024
Merged
4 tasks
@nojnhuh
Copy link
Contributor Author

nojnhuh commented Feb 8, 2024

At today's community meeting we agreed to start lazy consensus on this ending in one week. Please leave any last feedback soon!

@jackfrancis
Copy link
Contributor

/lgtm

Not every proposal lands with a consensus aye vote for a seventh option, nice work!

/approve

@nojnhuh let's release the hold after this week's community meeting

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 13, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: d0d199fe7f59447db0aa53d635a0c7b7d1190f08

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 13, 2024
mboersma
mboersma approved these changes Feb 15, 2024
View reviewed changes
Copy link
Contributor

@mboersma mboersma left a comment

Choose a reason for hiding this comment

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

/lgtm

7️⃣

@willie-yao
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 15, 2024
@nojnhuh
Copy link
Contributor Author

nojnhuh commented Feb 15, 2024

Broken link should be fixed now. PTAL.

/hold for squash

@jackfrancis
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 15, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 13a5e8010508b3bcc3b33c66230d8919b77e8d0d

@nojnhuh
Copy link
Contributor Author

nojnhuh commented Feb 15, 2024

squashed!
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 15, 2024
@k8s-ci-robot k8s-ci-robot merged commit bc5c8c3 into kubernetes-sigs:main Feb 15, 2024
18 checks passed
@nojnhuh nojnhuh deleted the auto-aks-proposal branch February 15, 2024 23:22
@nojnhuh nojnhuh mentioned this pull request Apr 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nawazkh nawazkh nawazkh left review comments

@matthchr matthchr matthchr left review comments

@CecileRobertMichon CecileRobertMichon CecileRobertMichon left review comments

@dtzar dtzar dtzar left review comments

@mjnovice mjnovice mjnovice left review comments

@AndiDog AndiDog AndiDog left review comments

@mtougeron mtougeron mtougeron left review comments

@jackfrancis jackfrancis Awaiting requested review from jackfrancis

@mboersma mboersma Awaiting requested review from mboersma

@willie-yao willie-yao Awaiting requested review from willie-yao

Assignees

@mboersma mboersma

@jackfrancis jackfrancis

@willie-yao willie-yao

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/proposal Issues or PRs related to proposals. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
CAPZ Planning
Archived in project
Milestone
v1.14
Development

Successfully merging this pull request may close these issues.