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

drop TCPRoute/UDPRoute controllers and RBAC #4166

Merged
merged 2 commits into from
Nov 10, 2021

Conversation

skriss
Copy link
Member

@skriss skriss commented Nov 9, 2021

With this PR, Contour no longer watches
TCPRoutes or UDPRoutes, and does not set
an "Accepted: false" condition on them.
All RBAC related to them is dropped as well.

Closes #4156.

Signed-off-by: Steve Kriss krisss@vmware.com

@skriss skriss requested a review from a team as a code owner November 9, 2021 16:14
@skriss skriss requested review from tsaarni and stevesloka and removed request for a team November 9, 2021 16:14
@skriss skriss added the release-note/small A small change that needs one line of explanation in the release notes. label Nov 9, 2021
@skriss
Copy link
Member Author

skriss commented Nov 9, 2021

One note here, the example manifests do still create the TCPRoute/UDPRoute CRDs, since we're just pulling directly from the Gateway API repo. This seems OK to me, but we could also add a script to remove them from the YAML entirely.

With this PR, Contour no longer watches
TCPRoutes or UDPRoutes, and does not set
an "Accepted: false" condition on them.
All RBAC related to them is dropped as well.

Closes projectcontour#4156.

Signed-off-by: Steve Kriss <krisss@vmware.com>
Signed-off-by: Steve Kriss <krisss@vmware.com>
@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #4166 (4b59e9b) into main (6d78450) will increase coverage by 1.95%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4166      +/-   ##
==========================================
+ Coverage   73.16%   75.11%   +1.95%     
==========================================
  Files         113      110       -3     
  Lines        9884     9630     -254     
==========================================
+ Hits         7232     7234       +2     
+ Misses       2498     2242     -256     
  Partials      154      154              
Impacted Files Coverage Δ
cmd/contour/serve.go 13.30% <ø> (+0.38%) ⬆️
internal/controller/gateway.go 0.00% <0.00%> (ø)
internal/sorter/sorter.go 98.78% <0.00%> (+1.21%) ⬆️

Copy link
Member

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

Removal looks fine, except for one question about the helpers.go file.

// See the License for the specific language governing permissions and
// limitations under the License.

package controller
Copy link
Member

Choose a reason for hiding this comment

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

Has this file been removed because we're not using it any more? How are the conditions for Gateway etc being set again?

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, not using it anymore -- I created this file when I added the TCPRoute/UDPRoute controllers to share some logic between them. Logic for setting GatewayClass/Gateway conditions are in the respective controllers, or in the DAG.

@stevesloka stevesloka merged commit 60c6f76 into projectcontour:main Nov 10, 2021
@skriss skriss deleted the rm-tcproute-udproute branch November 10, 2021 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/small A small change that needs one line of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop all TCPRoute/UDPRoute-related code
4 participants