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

feat: check server capabilities usin the preferred resources endpoint #54

Conversation

LucasRoesler
Copy link
Member

@LucasRoesler LucasRoesler commented Aug 29, 2021

Description

Using the preferred resources endpoit allows us to test for specific API
groups that contain a specific resource kind, in this case we can test
for which API groups that exist and support Ingress.

ci: remove git from the docker build context

Pass the versio data via build arguments instead of passing the entire
git database. This cuts the build context in half.

ci: update multi-arch build flow

Use the build process from faas-netes to simplify the multi-arch build
process.

ci: allow pushing to fork ghcr repos

ci: push preview images for PRs

ci: build multi-arch in paraller for speed

Motivation and Context

  • I have raised an issue to propose this change (required)

Closes #51
Closes #44
Improve #50

How Has This Been Tested?

Create the files cluster.yaml and test.yaml

# cluster.yaml
kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
nodes:
- role: control-plane
  kubeadmConfigPatches:
  - |
    kind: InitConfiguration
    nodeRegistration:
      kubeletExtraArgs:
        node-labels: "ingress-ready=true"
  extraPortMappings:
  - containerPort: 80
    hostPort: 80
    protocol: TCP
  - containerPort: 443
    hostPort: 443
    protocol: TCP
  - containerPort: 31112 # this is the NodePort created by the helm chart
    hostPort: 8080 # this is your port on localhost
    protocol: TCP
# test.yaml
apiVersion: openfaas.com/v1alpha2
kind: FunctionIngress
metadata:
  name: nodeinfo
  namespace: openfaas
spec:
  domain: "nodeinfo.myfaas.club"
  function: "nodeinfo"
  ingressType: "nginx"
  path: "/v1/profiles/(.*)"

Then, deploy a 1.19 cluster using

kind create cluster --image kindest/node:v1.19.11 --config=cluster.yaml

and install the test operator

$ kind create cluster --config=cluster.yaml
$ arkade install openfaas \
	--basic-auth=false \
	--clusterrole \
	--ingress-operator \
	--set ingressOperator.image=ghcr.io/lucasroesler/ingress-operator:pr-1


$ faas-cli store deploy nodeinfo
$ kubectl apply -f test.yaml
$ kubectl get ing
Warning: extensions/v1beta1 Ingress is deprecated in v1.14+, unavailable in v1.22+; use networking.k8s.io/v1 Ingress
NAME       CLASS    HOSTS                  ADDRESS   PORTS   AGE
nodeinfo   <none>   nodeinfo.myfaas.club             80      5s
$ kubectl -n openfaas logs deploy/ingress-operator 
I0829 09:49:55.758358       1 main.go:54] Starting FunctionIngress controller version: latest-dev commit: 56f035ce56c466f8b3b73cafd55dd735af933e58
W0829 09:49:55.758563       1 client_config.go:615] Neither --kubeconfig nor --master was specified.  Using the inClusterConfig.  This might not work.
I0829 09:49:55.770796       1 main.go:94] cluster supports ingress in: extensions/v1beta1, networking.k8s.io/v1
I0829 09:49:55.771156       1 controller.go:70] Setting up event handlers
I0829 09:49:55.771239       1 core.go:72] Waiting for informer caches to sync
I0829 09:49:55.872037       1 core.go:77] Starting workers
I0829 09:49:55.872053       1 core.go:83] Started workers
I0829 09:49:55.872087       1 controller.go:103] FunctionIngress name: nodeinfo
I0829 09:49:55.872098       1 controller.go:112] fni.Spec.UseTLS() false
I0829 09:49:55.872102       1 controller.go:113] createIngress false

Specifically take note of this log line:

I0829 09:49:55.770796       1 main.go:94] cluster supports ingress in: extensions/v1beta1, networking.k8s.io/v1

Deploy a 1.22 cluster using

kind create cluster --image kindest/node:v1.22.1 --config=cluster.yaml

Re-run the tests

$ kind create cluster --config=cluster.yaml
$ arkade install openfaas \
	--basic-auth=false \
	--clusterrole \
	--ingress-operator \
	--set ingressOperator.image=ghcr.io/lucasroesler/ingress-operator:pr-1


$ faas-cli store deploy nodeinfo
$ kubectl apply -f test.yaml
$ kubectl -n openfaas get ing
NAME       CLASS    HOSTS                  ADDRESS   PORTS   AGE
nodeinfo   <none>   nodeinfo.myfaas.club             80      5s

The logs now show only one supported API

$ kubectl -n openfaas logs deploy/ingress-operator
I0829 10:07:16.357689       1 main.go:54] Starting FunctionIngress controller version: latest-dev commit: 56f035ce56c466f8b3b73cafd55dd735af933e58
W0829 10:07:16.357836       1 client_config.go:615] Neither --kubeconfig nor --master was specified.  Using the inClusterConfig.  This might not work.
I0829 10:07:16.366955       1 main.go:94] cluster supports ingress in: networking.k8s.io/v1
I0829 10:07:16.367052       1 controller.go:70] Setting up event handlers
I0829 10:07:16.367089       1 core.go:72] Waiting for informer caches to sync
I0829 10:07:16.467864       1 core.go:77] Starting workers
I0829 10:07:16.467917       1 core.go:83] Started workers
I0829 10:07:43.764285       1 controller.go:103] FunctionIngress name: nodeinfo
I0829 10:07:43.764332       1 controller.go:112] fni.Spec.UseTLS() false
I0829 10:07:43.764338       1 controller.go:113] createIngress true

Note: I0829 10:07:16.366955 1 main.go:94] cluster supports ingress in: networking.k8s.io/v1

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Impact to existing users

None

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Using the preferred resources endpoit allows us to test for specific API
groups that contain a specific resource kind, in this case we can test
for which API groups that exist and support Ingress.

ci: remove git from the docker build context

Pass the versio data via build arguments instead of passing the entire
git database. This cuts the build context in half.

ci: update multi-arch build flow

Use the build process from faas-netes to simplify the multi-arch build
process.

ci: allow pushing to fork ghcr repos

ci: push preview images for PRs

ci: build multi-arch in paraller for speed

Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>
Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>
Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>
With the more fine-grained check, we can invert our preference and
safely test and use networking v1 instead of the v1beta1.

Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>
@LucasRoesler LucasRoesler marked this pull request as ready for review August 29, 2021 10:14
registry: ghcr.io
username: ${{ env.USER_REPO }}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% sure, but I think for me to kick off a release build and to have this work, the value of env.USER_REPO needs to equal my login account name.

The way to test is to create your own org and fork or move your repo there and kick off a build.

Copy link
Member Author

Choose a reason for hiding this comment

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

i tested it in a local fork with my work org, it did work, however, i think we should actually be using github.repository_owner

Here you can see the successful login in my fork
image

And here in the org repo
image

Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>
@LucasRoesler LucasRoesler force-pushed the feat-improve-cluster-capabilities-check branch from 9f78e6f to d000f2d Compare September 8, 2021 15:12
Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>
Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>
go-version: [1.13.x]
os: [ubuntu-latest]
runs-on: ${{ matrix.os }}
runs-on: ubuntu-latest
Copy link
Member

Choose a reason for hiding this comment

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

Please move to the publish as discussed on the call today.

Signed-off-by: Lucas Roesler <roesler.lucas@gmail.com>
Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

Approved, thank you for the fixes. Could you update the permissions for the release build please before merge?

@alexellis
Copy link
Member

I loved the testing instructions on this PR. I think you win the award for the best submitter of PRs in the openfaas org.

@alexellis alexellis merged commit 38a72f6 into openfaas:master Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants