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

Improve Project Layout and Refactor Controller Package #357

Merged
merged 10 commits into from
Sep 6, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ Session.vim
# Ingress Controller binaries
osx-nginx-ingress
nginx-ingress
!nginx-ingress/
osx-nginx-plus-ingress
nginx-plus-ingress
nginx-controller/nginx-controller
cmd/nginx-ic/nginx-ic
Copy link
Contributor

Choose a reason for hiding this comment

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

this line is not needed

Copy link
Author

Choose a reason for hiding this comment

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

it is needed as it is the binary produced from go build, but it should be nginx-ingress

Copy link
Contributor

Choose a reason for hiding this comment

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

it seems that the following lines cover that:

nginx-ingress
!nginx-ingress/


# NGINX Plus license files
*.crt
Expand All @@ -37,3 +38,6 @@ nginx-controller/nginx-controller

# Default certificate and key
default.pem

# Dockerfiles for building
Dockerfile
3 changes: 1 addition & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ go:
- "1.10"
script:
- echo "Building ingress controller commit:${TRAVIS_COMMIT}"
- cd nginx-controller &&
make BUILD_IN_CONTAINER=0 container;
- make BUILD_IN_CONTAINER=0 container;
before_install:
- echo "PR Slug:${TRAVIS_PULL_REQUEST_SLUG}"
- if [[ "${TRAVIS_PULL_REQUEST_SLUG}" == "nginxinc/kubernetes-ingress" || "${TRAVIS_PULL_REQUEST}" == "false" ]]; then
Expand Down
8 changes: 6 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@ Read the [documentation](https://github.com/nginxinc/kubernetes-ingress/tree/mas
### Project Structure

* This Ingress Controller is written in Go and supports both the open source NGINX software and NGINX Plus.
* The main code resides under `/nginx-controller`
* The project dependencies reside in the `/vendor`. We use [dep](https://github.com/golang/dep) for managing dependencies.
* The project follows a standard Go project layout
* The main code is found at `cmd/nginx-ic/`
Copy link
Contributor

Choose a reason for hiding this comment

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

the actual folder is cmd/nginx-ingress/

* The internal code is found at `internal/`
* Build files for Docker and CI are found under `build/`
* Deployment yaml files, and Helm files are found at `deployments/`
* The project dependencies are found at `vendor/`. We use [dep](https://github.com/golang/dep) for managing dependencies.

## Contributing

Expand Down
16 changes: 10 additions & 6 deletions nginx-controller/Makefile → Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,24 @@ VERSION = edge
TAG = $(VERSION)
PREFIX = nginx/nginx-ingress

DOCKER_RUN = docker run --rm -v $(shell pwd)/../:/go/src/github.com/nginxinc/kubernetes-ingress -w /go/src/github.com/nginxinc/kubernetes-ingress/nginx-controller/
DOCKER_RUN = docker run --rm -v $(shell pwd):/go/src/github.com/nginxinc/kubernetes-ingress
Copy link
Contributor

Choose a reason for hiding this comment

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

will DOCKER_TEST_RUN make more sense as a name?

Copy link
Author

Choose a reason for hiding this comment

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

I am not going to change this in this PR

DOCKER_BUILD_RUN = docker run --rm -v $(shell pwd):/go/src/github.com/nginxinc/kubernetes-ingress -w /go/src/github.com/nginxinc/kubernetes-ingress/cmd/nginx-ingress/
GOLANG_CONTAINER = golang:1.10
DOCKERFILEPATH = build
DOCKERFILE = Dockerfile

BUILD_IN_CONTAINER = 1
PUSH_TO_GCR =
GENERATE_DEFAULT_CERT_AND_KEY =
GENERATE_DEFAULT_CERT_AND_KEY =
DOCKER_BUILD_OPTIONS =

GIT_COMMIT=$(shell git rev-parse --short HEAD)

nginx-ingress:
ifeq ($(BUILD_IN_CONTAINER),1)
$(DOCKER_RUN) -e CGO_ENABLED=0 $(GOLANG_CONTAINER) go build -a -installsuffix cgo -ldflags "-w -X main.version=${VERSION} -X main.gitCommit=${GIT_COMMIT}" -o nginx-ingress *.go
$(DOCKER_BUILD_RUN) -e CGO_ENABLED=0 $(GOLANG_CONTAINER) go build -a -installsuffix cgo -ldflags "-w -X main.version=${VERSION} -X main.gitCommit=${GIT_COMMIT}" -o /go/src/github.com/nginxinc/kubernetes-ingress/nginx-ingress
else
CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -ldflags "-w -X main.version=${VERSION} -X main.gitCommit=${GIT_COMMIT}" -o nginx-ingress *.go
CGO_ENABLED=0 GOOS=linux go build -a -installsuffix cgo -ldflags "-w -X main.version=${VERSION} -X main.gitCommit=${GIT_COMMIT}" -o nginx-ingress github.com/nginxinc/kubernetes-ingress/cmd/nginx-ingress
endif

test:
Expand All @@ -29,12 +31,13 @@ else
go test ./...
endif

certificate-and-key:
certificate-and-key:
ifeq ($(GENERATE_DEFAULT_CERT_AND_KEY),1)
./generate_default_cert_and_key.sh
./build/generate_default_cert_and_key.sh
endif

container: test nginx-ingress certificate-and-key
cp $(DOCKERFILEPATH)/$(DOCKERFILE) .
docker build $(DOCKER_BUILD_OPTIONS) -f $(DOCKERFILE) -t $(PREFIX):$(TAG) .

push: container
Expand All @@ -46,3 +49,4 @@ endif

clean:
rm -f nginx-ingress
rm $(DOCKERFILE)
Copy link
Contributor

Choose a reason for hiding this comment

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

When make is called with DOCKERFILE=DockerfileForPlus, DockerfileForPlus is copied to /. As a result, make clean must also be called with DOCKERFILE=DockerfileForPlus. Perhaps we can copy DockerfileForPlus or any other as Dockerfile?

2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ NGINX Plus provides you with [advanced statistics](https://www.nginx.com/product
* **JWTs** NGINX Plus can validate JSON Web Tokens (JWTs), providing a flexible authentication mechanism.
* **Support** Support from NGINX Inc is available for NGINX Plus Ingress controller.

**Note**: Deployment of the Ingress controller for NGINX Plus requires you to do one extra step: build your own [Docker image](nginx-controller) using the certificate and key for your subscription.
**Note**: Deployment of the Ingress controller for NGINX Plus requires you to do one extra step: build your own [Docker image](build) using the certificate and key for your subscription.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also update 2 links for the edge version for build your own image in the NGINX Ingress Controller Releases section

Copy link
Contributor

Choose a reason for hiding this comment

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

links to the Helm chart as well in the same section

Copy link
Contributor

Choose a reason for hiding this comment

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

and a link to manifests in the same section

The Docker image of the Ingress controller for NGINX is [available on Docker Hub](https://hub.docker.com/r/nginx/nginx-ingress/).

## Using Multiple Ingress Controllers
Expand Down
2 changes: 1 addition & 1 deletion nginx-controller/Dockerfile → build/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ RUN ln -sf /proc/1/fd/1 /var/log/nginx/access.log \
&& ln -sf /proc/1/fd/1 /var/log/nginx/stream-access.log \
&& ln -sf /proc/1/fd/2 /var/log/nginx/error.log

COPY nginx-ingress nginx/templates/nginx.ingress.tmpl nginx/templates/nginx.tmpl /
COPY nginx-ingress internal/nginx/templates/nginx.ingress.tmpl internal/nginx/templates/nginx.tmpl /
Copy link
Contributor

Choose a reason for hiding this comment

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

DockerfileForAlpine and DockerfileForPlus require a similar update


RUN rm /etc/nginx/conf.d/*

Expand Down
File renamed without changes.
File renamed without changes.
9 changes: 3 additions & 6 deletions nginx-controller/README.md → build/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,13 @@ Although the Ingress controller is written in golang, golang is not required, as

### Building the Image and Pushing It to the Private Registry

We build the image using the make utility and the provided `Makefile`. Let’s create the controller binary, build an image and push the image to the private registry.
We build the image using the make utility and the provided `Makefile`. Let’s create the controller binary, build an image and push the image to the private registry.

1. Make sure to run the `docker login` command first to login to the registry. If you’re using Google Container Registry, you don’t need to use the docker command to login -- make sure you’re logged into the gcloud tool (using the `gcloud auth login` command) and set the variable `PUSH_TO_GCR=1` when running the make command.

1. Clone the Ingress controller repo and change your folder to `nginx-controller`:
1. Clone the Ingress controller repo:
```
$ git clone https://github.com/nginxinc/kubernetes-ingress/
$ cd kubernetes-ingress/nginx-controller
```

1. If you're using a stable release, check out the corresponding tag. For release 1.3.0, run:
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest removing this paragraph completely, cause it will check out the old structure

Expand Down Expand Up @@ -84,10 +83,8 @@ The **Makefile** contains the following main variables for you to customize (eit
1. `Dockerfile`, for building a debian-based image with NGINX. It's used by default.
1. `DockerfileForAlpine`, for building an alpine-based image with NGINX.
1. `DockerfileForPlus`, for building an debian-based image with NGINX Plus.
* **GENERATE_DEFAULT_CERT_AND_KEY** - The Ingress controller requires a certificate and a key for the default HTTP/HTTPS server. You can reference them in a TLS Secret in a command-line argument to the Ingress controller. As an alternative, you can add a file in the PEM format with your certificate and key to the image as `/etc/nginx/secrets/default`. Optionally, you can generate a self-signed certificate and a key during the build process. Set `GENERATE_DEFAULT_CERT_AND_KEY` to `1` to generate a certificate and a key in the `default.pem` file. Note that you must add the `ADD` instruction in the Dockerfile to copy the cert and the key to the image. The default value of `GENERATE_DEFAULT_CERT_AND_KEY` is `0`.
* **GENERATE_DEFAULT_CERT_AND_KEY** - The Ingress controller requires a certificate and a key for the default HTTP/HTTPS server. You can reference them in a TLS Secret in a command-line argument to the Ingress controller. As an alternative, you can add a file in the PEM format with your certificate and key to the image as `/etc/nginx/secrets/default`. Optionally, you can generate a self-signed certificate and a key during the build process. Set `GENERATE_DEFAULT_CERT_AND_KEY` to `1` to generate a certificate and a key in the `default.pem` file. Note that you must add the `ADD` instruction in the Dockerfile to copy the cert and the key to the image. The default value of `GENERATE_DEFAULT_CERT_AND_KEY` is `0`.
* **DOCKER_BUILD_OPTIONS** -- the [options](https://docs.docker.com/engine/reference/commandline/build/#options) for the `docker build` command. For example, `--pull`.
* **BUILD_IN_CONTAINER** -- By default, to compile the controller we use the [golang](https://hub.docker.com/_/golang/) container that we run as part of the building process. If you want to compile the controller using your local golang environment:
1. Make sure that the Ingress controller repo is in your `$GOPATH`.
1. Specify `BUILD_IN_CONTAINER=0` when you run the make command.


File renamed without changes.
66 changes: 48 additions & 18 deletions nginx-controller/main.go → cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ import (

"github.com/golang/glog"

"github.com/nginxinc/kubernetes-ingress/nginx-controller/controller"
"github.com/nginxinc/kubernetes-ingress/nginx-controller/nginx"
"github.com/nginxinc/kubernetes-ingress/nginx-controller/nginx/plus"
"github.com/nginxinc/kubernetes-ingress/internal/controller"
"github.com/nginxinc/kubernetes-ingress/internal/handlers"
"github.com/nginxinc/kubernetes-ingress/internal/nginx"
"github.com/nginxinc/kubernetes-ingress/internal/nginx/plus"
"github.com/nginxinc/kubernetes-ingress/internal/utils"
api_v1 "k8s.io/api/core/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -151,7 +153,7 @@ func main() {
ngxc := nginx.NewNginxController("/etc/nginx/", local)

if *defaultServerSecret != "" {
ns, name, err := controller.ParseNamespaceName(*defaultServerSecret)
ns, name, err := utils.ParseNamespaceName(*defaultServerSecret)
if err != nil {
glog.Fatalf("Error parsing the default-server-tls-secret argument: %v", err)
}
Expand All @@ -175,7 +177,7 @@ func main() {

cfg := nginx.NewDefaultConfig()
if *nginxConfigMaps != "" {
ns, name, err := controller.ParseNamespaceName(*nginxConfigMaps)
ns, name, err := utils.ParseNamespaceName(*nginxConfigMaps)
if err != nil {
glog.Fatalf("Error parsing the nginx-configmaps argument: %v", err)
}
Expand Down Expand Up @@ -230,21 +232,49 @@ func main() {
controllerNamespace := os.Getenv("POD_NAMESPACE")

lbcInput := controller.NewLoadBalancerControllerInput{
KubeClient: kubeClient,
ResyncPeriod: 30 * time.Second,
Namespace: *watchNamespace,
CNF: cnf,
NginxConfigMaps: *nginxConfigMaps,
DefaultServerSecret: *defaultServerSecret,
NginxPlus: *nginxPlus,
IngressClass: *ingressClass,
UseIngressClassOnly: *useIngressClassOnly,
ExternalServiceName: *externalService,
ControllerNamespace: controllerNamespace,
ReportIngressStatus: *reportIngressStatus,
LeaderElectionEnabled: *leaderElectionEnabled,
KubeClient: kubeClient,
ResyncPeriod: 30 * time.Second,
Namespace: *watchNamespace,
NginxConfigurator: cnf,
DefaultServerSecret: *defaultServerSecret,
IsNginxPlus: *nginxPlus,
IngressClass: *ingressClass,
UseIngressClassOnly: *useIngressClassOnly,
ExternalServiceName: *externalService,
ControllerNamespace: controllerNamespace,
ReportIngressStatus: *reportIngressStatus,
IsLeaderElectionEnabled: *leaderElectionEnabled,
}

lbc := controller.NewLoadBalancerController(lbcInput)

// create handlers for resources we care about
ingressHandlers := handlers.CreateIngressHandlers(lbc)
secretHandlers := handlers.CreateSecretHandlers(lbc)
serviceHandlers := handlers.CreateServiceHandlers(lbc)
endpointHandlers := handlers.CreateEndpointHandlers(lbc)

lbc.AddSecretHandler(secretHandlers)
lbc.AddIngressHandler(ingressHandlers)
lbc.AddServiceHandler(serviceHandlers)
lbc.AddEndpointHandler(endpointHandlers)

if *nginxConfigMaps != "" {
nginxConfigMapsNS, nginxConfigMapsName, err := utils.ParseNamespaceName(*nginxConfigMaps)
if err != nil {
glog.Warning(err)
} else {
lbc.WatchNginxConfigMaps()
configMapHandlers := handlers.CreateConfigMapHandlers(lbc, nginxConfigMapsName)
lbc.AddConfigMapHandler(configMapHandlers, nginxConfigMapsNS)
}
}

if lbcInput.ReportIngressStatus && lbcInput.IsLeaderElectionEnabled {
leaderHandler := handlers.CreateLeaderHandler(lbc)
lbc.AddLeaderHandler(leaderHandler)
}

go handleTermination(lbc, ngxc, nginxDone)
lbc.Run()

Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion helm-chart/README.md → deployments/helm-chart/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ This chart deploys the NGINX Ingress controller in your Kubernetes cluster.
- Helm 2.8.x+.
- Git.
- If you’d like to use NGINX Plus:
- Build an Ingress controller image with NGINX Plus and push it to your private registry by following the instructions from [here](../nginx-controller/README.md).
- Build an Ingress controller image with NGINX Plus and push it to your private registry by following the instructions from [here](../../build/README.md).
- Update the `controller.image.repository` field of the `values-plus.yaml` accordingly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also remove If you're using a stable release, check out the corresponding tag. For release 1.3.0 paragraph.

Also, update Change your working directory to /helm-chart, cd kubernetes-ingress/helm-chart x2


## Installing the Chart
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion docs/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
Make sure you have access to the Ingress controller image:

* For NGINX Ingress controller, use the image `nginx/nginx-ingress` from [DockerHub](https://hub.docker.com/r/nginx/nginx-ingress/).
* For NGINX Plus Ingress controller, build your own image and push it to your private Docker registry by following the instructions from [here](../nginx-controller).
* For NGINX Plus Ingress controller, build your own image and push it to your private Docker registry by following the instructions from [here](../build/README.md).

The installation manifests are located in the [install](../install) folder. In the steps below we assume that you will be running the commands from that folder.
Copy link
Contributor

Choose a reason for hiding this comment

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

please rename install to deployments


Expand Down
2 changes: 1 addition & 1 deletion docs/nginx-ingress-controllers.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ The table below summarizes the key difference between nginxinc/kubernetes-ingres
| TCP SSL Passthrough | Supported via a ConfigMap | Not supported | Not supported |
| JWT validation | Not supported | Not supported | Supported |
| Session persistence | Supported via a third-party module | Not supported | Supported |
| Configuration templates *1 | See the [template](https://github.com/kubernetes/ingress-nginx/blob/master/rootfs/etc/nginx/template/nginx.tmpl) | See the [templates](https://github.com/nginxinc/kubernetes-ingress/tree/master/nginx-controller/nginx/templates) | See the [templates](https://github.com/nginxinc/kubernetes-ingress/tree/master/nginx-controller/nginx/templates) |
| Configuration templates *1 | See the [template](https://github.com/kubernetes/ingress-nginx/blob/master/rootfs/etc/nginx/template/nginx.tmpl) | See the [templates](https://github.com/nginxinc/kubernetes-ingress/tree/master/internal/nginx/templates) | See the [templates](https://github.com/nginxinc/kubernetes-ingress/tree/master/internal/nginx/templates) |
| **Deployment** |
| Command-line arguments *2 | See the [arguments](https://github.com/kubernetes/ingress-nginx/blob/master/docs/user-guide/cli-arguments.md) | See the [arguments](cli-arguments.md) | See the [arguments](cli-arguments.md) |
| TLS certificate and key for the default server | Required as a command-line argument/ auto-generated | Required as a command-line argument | Required as a command-line argument |
Expand Down
12 changes: 6 additions & 6 deletions examples/openshift/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@

## Prerequisites

* A cluster with OpenShift Release 3.5 and above.
* A cluster with OpenShift Release 3.5 and above.
* You must be the cluster administrator to deploy the Ingress controller.
* For NGINX Plus:
* Build and make available in your cluster the [Ingress controller](../../nginx-controller) image.
* Build and make available in your cluster the [Ingress controller](../../build) image.
* Update the container image field in the `nginx-plus-ingress-rc.yaml` file accordingly.


## Steps

1. Avoid conflicts with the OpenShift Router.

NGINX Plus Ingress controller must be able to bind to ports 80 and 443 of the cluster node, where it is running, like the OpenShift Router. Thus, you need to make sure that the Ingress controller and the Router are running on separate nodes. Additionally, NGINX Plus binds to port 8080 to expose its API and the monitoring dashboard.

To quickly disable the Router you can run:
Expand All @@ -27,7 +27,7 @@

1. Create a service account for the Ingress controller with the name *nginx-ingress*:
```
$ oc create sa nginx-ingress
$ oc create sa nginx-ingress
```
1. Create a cluster role for the Ingress controller:
```
Expand Down Expand Up @@ -62,7 +62,7 @@
$ oc create -f ingress-admin-role.yaml
```

Add this role to the users. As an example, we add this role for the user *developer* from the project *myproject*.
Add this role to the users. As an example, we add this role for the user *developer* from the project *myproject*.
```
$ oc policy add-role-to-user ingress-admin developer -n myproject
```
Expand All @@ -71,4 +71,4 @@
$ oc adm policy add-scc-to-user anyuid -z default -n myproject
```

1. Now you can login as the user *developer* to the project *myproject* and [deploy the Cafe application](../complete-example#2-deploy-the-cafe-application).
1. Now you can login as the user *developer* to the project *myproject* and [deploy the Cafe application](../complete-example#2-deploy-the-cafe-application).
Loading