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

Enable CRDs by default #779

Merged
merged 3 commits into from
Dec 5, 2019
Merged

Enable CRDs by default #779

merged 3 commits into from
Dec 5, 2019

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.

enableCustomResources = flag.Bool("enable-custom-resources", false,
"Enable custom resources")

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.

docs/installation.md Show resolved Hide resolved
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.

3 participants