Skip to content

Conversation

ivankatliarchuk
Copy link
Contributor

@ivankatliarchuk ivankatliarchuk commented Feb 9, 2025

Description

Relates #4566
Related pull request #4574
Another related pull request #4808

We either having a plan to rollback, or have a clear answer that we not planning to rollback/change this behaviour

The proposal to merge to master 2025-Mar-09 with the decision

Checklist

  • Unit tests updated
  • End user documentation updated

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 9, 2025
@ivankatliarchuk
Copy link
Contributor Author

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Feb 9, 2025
@szuecs
Copy link
Contributor

szuecs commented Feb 18, 2025

/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 18, 2025
Comment on lines 32 to 34
- ***Propose/Add an annotation for this specific use case***
- Provide support for `external-dns.alpha.kubernetes.io/expose-internal-ipv6` in follow-up releases.
- Managing dual annotation and flag may introduce complexity.
Copy link
Collaborator

@mloiseleur mloiseleur Feb 18, 2025

Choose a reason for hiding this comment

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

Suggested change
- ***Propose/Add an annotation for this specific use case***
- Provide support for `external-dns.alpha.kubernetes.io/expose-internal-ipv6` in follow-up releases.
- Managing dual annotation and flag may introduce complexity.

FTM, I think I have a strong opinion against this annotation with its induced complexity.
My head hurts just trying to imagine a use case requiring this level of complexity.
Please let me know if you think you have a good one, otherwise I suggest to avoid this complexity, at least FTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to non-goal section. Basically it was based on issue and from discussion. I have no strong opinion on that stuff.

Copy link
Contributor Author

@ivankatliarchuk ivankatliarchuk Feb 19, 2025

Choose a reason for hiding this comment

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

Just for reference, when it was proposed by maintainer

But reading more thread, could be a bit controversial

* master: (31 commits)
  fix(source): debug log on gateway target detection
  Update docs/sources/service.md
  chore(formatting): fix infected files with correct formatting (kubernetes-sigs#5099)
  docs: Fix managed-record-type argument
  Update docs/sources/service.md
  docs(proposal): support multiple replicas with leader election (kubernetes-sigs#5051)
  fixed golangci-lint config
  updated MD files
  updated MD files
  feat(chart): automate helm json schema (kubernetes-sigs#5075)
  docs(proposal): update proposal template, add statuses
  test(aws): introduce first fixture-based (kubernetes-sigs#5092)
  chore(makefile): add helper and document targets
  feat: Updated chart for v1.15.2 release
  chore(makefile): add helper and document targets
  chore(filter-tags): pre-process filter tags
  chore(filter-tags): pre-process filter tags
  chore(filter-tags): pre-process filter tags
  chore(deps): bump the dev-dependencies group across 1 directory with 21 updates
  update service.md, service.go
  ...
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 19, 2025
@mloiseleur
Copy link
Collaborator

/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 19, 2025
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 27, 2025
@ivankatliarchuk
Copy link
Contributor Author

Lost approval, changes

  • fixed date
  • added participants, approvers to list of authors

@mloiseleur
Copy link
Collaborator

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mloiseleur

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 27, 2025
@k8s-ci-robot k8s-ci-robot merged commit d114e06 into kubernetes-sigs:master Feb 27, 2025
13 checks passed
ivankatliarchuk added a commit to gofogo/k8s-sigs-external-dns-fork that referenced this pull request Mar 6, 2025
* master: (198 commits)
  fix(aws-sd): service instances registration and deregistration (kubernetes-sigs#5135)
  chore(docs): generate docs/monitoring/metrics.md file (kubernetes-sigs#5117)
  feat(chart): add helm-unittest framework (kubernetes-sigs#5137)
  feat(chart): add helm-unittest framework
  feat(aws): always create AAAA alias records in route53 (kubernetes-sigs#5111)
  feat(aws): fetch zones with tags batching (kubernetes-sigs#5058)
  docs: openwrt webhook (kubernetes-sigs#5132)
  docs(proposal): ipv6 internal node ip rollback plan (kubernetes-sigs#5081)
  docs(proposal): update date format
  chore(deps): bump the dev-dependencies group across 1 directory with 7 updates
  Update README.md with proper link to dev guide
  Add OpenStack Designate webook provider to readme
  chore(deps): bump the dev-dependencies group with 3 updates
  chore(deps): bump the dev-dependencies group with 20 updates
  chore(deps): bump azure/setup-helm in the dev-dependencies group
  style: formatting
  fix: remove broken test
  fix test name
  chore: upgrade ExternalDNS to go 1.24
  chore-makefile-coverage
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants