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

Modify URL parsing for advertise-urls used by etcd sidecar #715

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Modify URL parsing for advertise-urls used by etcd sidecar #715

wants to merge 6 commits into from
+308 −119

Conversation

anveshreddy18
Copy link
Contributor

@anveshreddy18 anveshreddy18 commented Feb 27, 2024 •
edited
Loading

What this PR does / why we need it:

This PR modifies the parsing logic for the etcd config parameters initial-advertise-peer-urls and advertise-client-urls as the druid PR #812 now uses proper URLs for these flags instead of @ as separator currently being used.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

Enhances parsing logic for Etcd Config parameters `initial-advertise-peer-urls` and `advertise-client-urls` to support proper URL formatting

@anveshreddy18 anveshreddy18 requested a review from a team as a code owner February 27, 2024 11:39
@gardener-robot gardener-robot added needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Feb 27, 2024
@anveshreddy18 anveshreddy18 added kind/enhancement Enhancement, improvement, extension area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/cleanup Something that is not needed anymore and can be cleaned up and removed size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Feb 27, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 27, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 27, 2024
@anveshreddy18 anveshreddy18 mentioned this pull request Feb 27, 2024
Closed
3 tasks
@anveshreddy18 anveshreddy18 self-assigned this Feb 27, 2024
@shreyas-s-rao shreyas-s-rao added this to the v0.30.0 milestone Mar 26, 2024
@anveshreddy18 anveshreddy18 modified the milestones: v0.30.0, v0.31.0 Aug 21, 2024
@gardener-robot gardener-robot added the size/s Size of pull request is small (see gardener-robot robot/bots/size.py) label Sep 2, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 2, 2024
@anveshreddy18 anveshreddy18 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 9, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 9, 2024
@anveshreddy18 anveshreddy18 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 11, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 11, 2024
renormalize
renormalize requested changes Sep 12, 2024
View reviewed changes
Copy link
Member

@renormalize renormalize left a comment

Choose a reason for hiding this comment

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

Thanks for your PR @anveshreddy18!

I'm personally not a fan of regular expressions so I've suggested a different way of doing it in my review. If you don't have any objections and think this method is clearer, please go ahead and adapt these changes to the two functions that are using regex.

Thanks.

pkg/miscellaneous/miscellaneous.go Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Sep 12, 2024
@renormalize renormalize mentioned this pull request Sep 24, 2024
Open
3 tasks
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 24, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 24, 2024
@seshachalam-yv
Copy link
Contributor

seshachalam-yv commented Sep 26, 2024 •
edited
Loading

@anveshreddy18
From the perspective of etcd-backup-restore, there is no ConfigMap involved. It should be referred to as a config file. Can you update the release note ?

@gardener-robot gardener-robot added size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else and removed size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Oct 8, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 8, 2024
@ishan16696 ishan16696 self-assigned this Oct 17, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 17, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Oct 17, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 21, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 21, 2024
ishan16696
ishan16696 requested changes Oct 24, 2024
View reviewed changes
Copy link
Member

@ishan16696 ishan16696 left a comment

Choose a reason for hiding this comment

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

Can you also update the charts: here and here

pkg/server/backuprestoreserver.go Outdated Show resolved Hide resolved
pkg/miscellaneous/testdata/valid_config.yaml Outdated Show resolved Hide resolved
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 24, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 24, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ishan16696 ishan16696 ishan16696 requested changes

@seshachalam-yv seshachalam-yv seshachalam-yv requested changes

@renormalize renormalize renormalize approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

@ishan16696 ishan16696

@anveshreddy18 anveshreddy18

@seshachalam-yv seshachalam-yv

Labels
area/quality Output qualification (tests, checks, scans, automation in general, etc.) related kind/cleanup Something that is not needed anymore and can be cleaned up kind/enhancement Enhancement, improvement, extension needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review needs/second-opinion Needs second review by someone else size/l Size of pull request is large (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
v0.31.0
Development

Successfully merging this pull request may close these issues.
9 participants
@anveshreddy18 @seshachalam-yv @ishan16696 @renormalize @gardener-robot-ci-1 @shreyas-s-rao @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot