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

internal/dag: Remove Ingress v1beta1 #3645

Merged

Conversation

sunjayBhatia
Copy link
Member

  • Removes v1beta1 tests
  • Removes v1beta1 Ingress insertion/deletion from cache
    • Note: This will mean k8s pre-1.19 will stop working with Ingress
      v1beta1 since Ingress v1 does not exist yet in those versions

Updates: #3628

@sunjayBhatia sunjayBhatia requested a review from a team as a code owner May 6, 2021 19:57
@sunjayBhatia sunjayBhatia requested review from stevesloka and youngnick and removed request for a team May 6, 2021 19:57
@sunjayBhatia sunjayBhatia added this to the 1.16.0 milestone May 6, 2021
@sunjayBhatia sunjayBhatia added this to 1.16 release in Contour Project Board May 6, 2021
@sunjayBhatia sunjayBhatia added release-note Denotes a PR that will be considered when it comes time to generate release notes. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. labels May 6, 2021
@sunjayBhatia sunjayBhatia marked this pull request as draft May 6, 2021 20:01
@sunjayBhatia
Copy link
Member Author

Converted to draft b/c this package refactor relies on others

@sunjayBhatia
Copy link
Member Author

sunjayBhatia commented May 7, 2021

added release note labels because this will make Contour unusable in versions of k8s that do not have ingress v1, as we're no longer falling back to v1beta1

- Removes v1beta1 tests
- Removes v1beta1 Ingress insertion/deletion from cache
  - Note: This will mean k8s pre-1.19 will stop working with Ingress
    v1beta1 since Ingress v1 does not exist yet in those versions

Updates: projectcontour#3628

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia sunjayBhatia marked this pull request as ready for review May 11, 2021 15:33
@sunjayBhatia sunjayBhatia requested a review from skriss May 11, 2021 15:33
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@codecov
Copy link

codecov bot commented May 11, 2021

Codecov Report

Merging #3645 (555dd90) into main (5635b97) will decrease coverage by 0.22%.
The diff coverage is n/a.

❗ Current head 555dd90 differs from pull request most recent head e9caaa1. Consider uploading reports for the commit e9caaa1 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3645      +/-   ##
==========================================
- Coverage   77.08%   76.85%   -0.23%     
==========================================
  Files         100      100              
  Lines        7121     7056      -65     
==========================================
- Hits         5489     5423      -66     
- Misses       1511     1513       +2     
+ Partials      121      120       -1     
Impacted Files Coverage Δ
internal/dag/cache.go 95.48% <ø> (-0.25%) ⬇️
internal/k8s/log.go 63.04% <0.00%> (-6.53%) ⬇️

…1-dag

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -3080,777 +3079,575 @@ func TestDAGInsert(t *testing.T) {
},
}

i1 := &v1beta1.Ingress{
// s3a and b have http/2 protocol annotations
Copy link
Member

Choose a reason for hiding this comment

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

oof, this file's diff is pretty hard to review, but tests are passing so 🤞

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, i think its maybe best to look at with the split diff type rather than the unified one, should just mostly be removals

@stevesloka stevesloka merged commit 5b5a1e6 into projectcontour:main May 11, 2021
Contour Project Board automation moved this from 1.16 release to 1.14 release completed May 11, 2021
@sunjayBhatia sunjayBhatia deleted the remove-ingress-v1beta1-dag branch May 11, 2021 19:54
@youngnick youngnick moved this from 1.14 release completed to 1.16 release in Contour Project Board May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action.
Projects
No open projects
Contour Project Board
  
1.16 release (completed)
Development

Successfully merging this pull request may close these issues.

None yet

3 participants