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

cleanup: remove kube-proxy-images.json #4556

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

jason1028kr
Copy link
Collaborator

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
remove kube-proxy-image.json
Which issue(s) this PR fixes:

Fixes #

Requirements:

Special notes for your reviewer:

Release note:

none

@coveralls
Copy link

coveralls commented Jun 26, 2024

Pull Request Test Coverage Report for Build 9687522233

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.486%

Totals Coverage Status
Change from base Build 9674880013: 0.0%
Covered Lines: 2627
Relevant Lines: 3727

💛 - Coveralls

@jason1028kr jason1028kr changed the title chore: remove kube-proxy.json cleanup: remove kube-proxy.json Jun 26, 2024
@coveralls
Copy link

coveralls commented Jun 26, 2024

Pull Request Test Coverage Report for Build 9687550537

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.486%

Totals Coverage Status
Change from base Build 9674880013: 0.0%
Covered Lines: 2627
Relevant Lines: 3727

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 26, 2024

Pull Request Test Coverage Report for Build 9687655292

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.486%

Totals Coverage Status
Change from base Build 9674880013: 0.0%
Covered Lines: 2627
Relevant Lines: 3727

💛 - Coveralls

@@ -381,6 +381,29 @@
"multiArchVersions": [
"v0.7.1"
]
},
{
"downloadURL": "mcr.microsoft.com/oss/kubernetes/kube-proxy:*",
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't seem like we're actually using this during VHD builds (install-dependencies.sh), are we not supposed to be caching this anymore?

Copy link
Collaborator

Choose a reason for hiding this comment

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

confirmed offline that we still aim to cache this, should probably wait on merging this until after @Devinwong's change merge into master

Copy link
Collaborator Author

@jason1028kr jason1028kr Jun 27, 2024

Choose a reason for hiding this comment

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

Wouldn't this loop https://github.com/Azure/AgentBaker/pull/4556/files#diff-d6aeb8e845af4c32251915a17dc7c887d73cd628262e83118b9dcd65f7728c98L312-R343 cover the added json entry under 'containerImages' key? I couldn't properly check b/c test pipeline was failing for other reasons, but I'll confirm once that's unblocked. I can also just pause until schema's finalized as you suggested.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, yes you're right :)

approved, up to you whether you wanna merge now before Devin's changes or wait until the schema is updated

@coveralls
Copy link

coveralls commented Jun 28, 2024

Pull Request Test Coverage Report for Build 9716172770

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.486%

Totals Coverage Status
Change from base Build 9708280130: 0.0%
Covered Lines: 2627
Relevant Lines: 3727

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jul 1, 2024

Pull Request Test Coverage Report for Build 9748402800

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.486%

Totals Coverage Status
Change from base Build 9748032807: 0.0%
Covered Lines: 2627
Relevant Lines: 3727

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jul 2, 2024

Pull Request Test Coverage Report for Build 9753410800

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.486%

Totals Coverage Status
Change from base Build 9753173800: 0.0%
Covered Lines: 2627
Relevant Lines: 3727

💛 - Coveralls

{
"downloadURL": "mcr.microsoft.com/oss/kubernetes/kube-proxy:*",
"amd64OnlyVersions": [],
"multiArchVersions": [
Copy link
Contributor

Choose a reason for hiding this comment

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

that's way too may version. after we review the Schema, we will need to keep 3 images only (one per AKS versions)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe we'll need to keep corresponding kube-proxy version for each unique major.minor.patch k8s version

@coveralls
Copy link

coveralls commented Jul 5, 2024

Pull Request Test Coverage Report for Build 9814673025

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.486%

Totals Coverage Status
Change from base Build 9811870560: 0.0%
Covered Lines: 2627
Relevant Lines: 3727

💛 - Coveralls

@jason1028kr jason1028kr changed the title cleanup: remove kube-proxy.json cleanup: remove kube-proxy-images.json Jul 12, 2024
@coveralls
Copy link

coveralls commented Jul 12, 2024

Pull Request Test Coverage Report for Build 9961922027

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 71.021%

Totals Coverage Status
Change from base Build 9961859337: 0.0%
Covered Lines: 2566
Relevant Lines: 3613

💛 - Coveralls

@jason1028kr jason1028kr merged commit edd575d into master Jul 16, 2024
19 checks passed
@jason1028kr jason1028kr deleted the jasonjung/migrate-k8s-proxy branch July 16, 2024 19:40
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.

None yet

5 participants