Skip to content

Conversation

@Dean-Coakley
Copy link
Contributor

Proposed changes

CRDs were made GA in #772

CRDs should now be enabled by default in our provided manifests and helm chart.

Checklist

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

  • I have read the CONTRIBUTING doc
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

@Dean-Coakley Dean-Coakley added proposal An issue that proposes a feature request setup labels Nov 29, 2019
@Dean-Coakley Dean-Coakley self-assigned this Nov 29, 2019
Copy link
Contributor Author

@Dean-Coakley Dean-Coakley left a comment

Choose a reason for hiding this comment

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

I could also leave manifests with - enable-custom-resources=false commented out and make the flag default to true if that make more sense.

https://github.com/nginxinc/kubernetes-ingress/blob/de8d38465db60f76436a8c391096b296670baa7b/cmd/nginx-ingress/main.go#L127-L128

Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

@Dean-Coakley thx. Please update the main.go https://github.com/nginxinc/kubernetes-ingress/blob/master/cmd/nginx-ingress/main.go#L127 and change the default value to true for the -enable-custom-resources flag. Make sure to update CLI args docs as well. Also, this means you don't need to specify -enable-custom-resources in the manifests because it will be true by default.

Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

👍

@Dean-Coakley Dean-Coakley merged commit ca5776e into master Dec 5, 2019
@Dean-Coakley Dean-Coakley deleted the make-crds-default branch December 5, 2019 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

proposal An issue that proposes a feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants