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 support for backup directive for VS and TS #4653

Merged
merged 59 commits into from
Dec 12, 2023
Merged

Conversation

jjngx
Copy link
Contributor

@jjngx jjngx commented Nov 15, 2023

Proposed changes

This PR introduces support for backup directive (external name service) for Transport Server and Virtual Server for NIC Plus.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@github-actions github-actions bot added the enhancement Pull requests for new features/feature enhancements label Nov 15, 2023
@jjngx jjngx linked an issue Nov 15, 2023 that may be closed by this pull request
@jjngx jjngx linked an issue Nov 15, 2023 that may be closed by this pull request
Copy link

codecov bot commented Nov 15, 2023

Codecov Report

Attention: 61 lines in your changes are missing coverage. Please review.

Comparison is base (5e0bd5c) 51.96% compared to head (9942a71) 52.00%.

Files Patch % Lines
internal/k8s/controller.go 0.00% 30 Missing ⚠️
internal/configs/virtualserver.go 56.52% 15 Missing and 5 partials ⚠️
internal/configs/transportserver.go 68.00% 6 Missing and 2 partials ⚠️
pkg/apis/configuration/validation/virtualserver.go 91.89% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4653      +/-   ##
==========================================
+ Coverage   51.96%   52.00%   +0.03%     
==========================================
  Files          60       60              
  Lines       17191    17317     +126     
==========================================
+ Hits         8933     9005      +72     
- Misses       7954     7999      +45     
- Partials      304      313       +9     

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

@jjngx jjngx marked this pull request as ready for review December 4, 2023 21:27
@jjngx jjngx requested a review from a team as a code owner December 4, 2023 21:27
@jjngx jjngx changed the title WIP - backup service Add support for backup service for VS and TS Dec 4, 2023
@jjngx jjngx changed the title Add support for backup service for VS and TS Add support for backup directive for VS and TS Dec 4, 2023
@jjngx jjngx requested a review from a team as a code owner December 5, 2023 14:47
@github-actions github-actions bot added the documentation Pull requests/issues for documentation label Dec 5, 2023
Copy link
Contributor

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

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

LGTM still though I made a number of suggestions in the example README.

I think I got all instances, but for clarity:

  • We only need to prepend F5 to the very first mention of NGINX Ingress Controller in a given piece of documentation.
  • If we're referring to it as a product or a single instance, we just say "NGINX Ingress Controller": no need to prefix it with "the".

There's some other changes that could be made, but none are blocking, and I will make them myself in the future as part of reviewing the writing in the examples folder.

jjngx and others added 10 commits December 12, 2023 13:37
…ADME.md

Co-authored-by: Alan Dooley <ADubhlaoich@users.noreply.github.com>
Signed-off-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com>
…ADME.md

Co-authored-by: Alan Dooley <ADubhlaoich@users.noreply.github.com>
Signed-off-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com>
Co-authored-by: Venktesh Shivam Patel <ve.patel@f5.com>
Signed-off-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com>
…e-resources.md

Co-authored-by: Venktesh Shivam Patel <ve.patel@f5.com>
Signed-off-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com>
…ADME.md

Co-authored-by: Alan Dooley <ADubhlaoich@users.noreply.github.com>
Signed-off-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com>
…ADME.md

Co-authored-by: Alan Dooley <ADubhlaoich@users.noreply.github.com>
Signed-off-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com>
…ADME.md

Co-authored-by: Alan Dooley <ADubhlaoich@users.noreply.github.com>
Signed-off-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com>
…ADME.md

Co-authored-by: Alan Dooley <ADubhlaoich@users.noreply.github.com>
Signed-off-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com>
…ADME.md

Co-authored-by: Alan Dooley <ADubhlaoich@users.noreply.github.com>
Signed-off-by: Jakub Jarosz <99677300+jjngx@users.noreply.github.com>
@github-actions github-actions bot added the tests Pull requests that update tests label Dec 12, 2023
@jjngx jjngx merged commit ba01bc1 into main Dec 12, 2023
67 checks passed
@jjngx jjngx deleted the feat/backup-service branch December 12, 2023 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements tests Pull requests that update tests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Support Upstream:Backup directive for service failure use cases
7 participants