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

knative generated files are not compliant to the new knative protocol #8659

Closed
mgohashi opened this issue Apr 17, 2020 · 3 comments
Closed
Assignees
Labels
kind/bug Something isn't working
Milestone

Comments

@mgohashi
Copy link

Describe the bug
The files generated by the openshift extension is not compliant to the new knative protocol

Expected behavior
Generate files compliant with the protocol including renaming standard 8080 ports from http to http1 or h2c and remove ports from liveness and readiness probes. For reference [1]

And this is the resulting yaml file:

spec:
  template:
    spec:
      containers:
      - image: "image-registry.openshift-image-registry.svc:5000/knative-app/quickstart-knative:1.0-SNAPSHOT"
        imagePullPolicy: "Always"
        livenessProbe:
          failureThreshold: 3
          httpGet:
            path: "/health/live"
            scheme: "HTTP"
          initialDelaySeconds: 0
          periodSeconds: 30
          successThreshold: 1
          timeoutSeconds: 10
        name: "quickstart-knative"
        ports:
        - containerPort: 8080
          protocol: "TCP"
        readinessProbe:
          failureThreshold: 3
          httpGet:
            path: "/health/ready"
            scheme: "HTTP"
          initialDelaySeconds: 0
          periodSeconds: 30
          successThreshold: 1
          timeoutSeconds: 10

[1] https://github.com/knative/serving/blob/master/docs/runtime-contract.md#protocols-and-ports

Actual behavior
The extension is generating the following yaml

spec:
  template:
    spec:
      containers:
      - image: "image-registry.openshift-image-registry.svc:5000/knative-app/quickstart-knative:1.0-SNAPSHOT"
        imagePullPolicy: "Always"
        livenessProbe:
          failureThreshold: 3
          httpGet:
            path: "/health/live"
            port: 8080
            scheme: "HTTP"
          initialDelaySeconds: 0
          periodSeconds: 30
          successThreshold: 1
          timeoutSeconds: 10
        name: "quickstart-knative"
        ports:
        - containerPort: 8080
          name: "http"
          protocol: "TCP"
        readinessProbe:
          failureThreshold: 3
          httpGet:
            path: "/health/ready"
            port: 8080
            scheme: "HTTP"
          initialDelaySeconds: 0
          periodSeconds: 30
          successThreshold: 1
          timeoutSeconds: 10

To Reproduce
Steps to reproduce the behavior:

  1. Create an empty app
  2. Add openshift, smallrye-heath, resteasy-jsonb extensions
  3. Run ./mvnw clean package
  4. Look at the generated file: ./target/kubernetes/knative.yml

Configuration

# Add your application.properties here, if applicable.
quarkus.knative.containers.quickstart-knative.ports.http1.container-port=8080
quarkus.openshift.containers.quickstart-knative.ports.http1.container-port=8080

Screenshots
N/A

Environment (please complete the following information):

  • Output of uname -a or ver: Linux mohashi-pro.local 5.5.15-200.fc31.x86_64 Switch to the Maven distributed copy of the SubstrateVM annotations #1 SMP Thu Apr 2 19:16:17 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
  • Output of java -version: openjdk version "1.8.0_242"
    OpenJDK Runtime Environment (build 1.8.0_242-b08)
    OpenJDK 64-Bit Server VM (build 25.242-b08, mixed mode)
  • GraalVM version (if different from Java):
  • Quarkus version or git rev: 1.3.2.Final
  • Build tool (ie. output of mvnw --version or gradlew --version): Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)

Additional context
N/A

@mgohashi mgohashi added the kind/bug Something isn't working label Apr 17, 2020
@geoand
Copy link
Contributor

geoand commented Apr 18, 2020

@iocanel can you please take a look?

@iocanel
Copy link
Contributor

iocanel commented May 11, 2020

Dekorate now recognizes http1 and h2c as names referring to web ports, so if the user selects them it should work with no issues.

On top of that we need extra config options, to make selection of one or the other easier.

@iocanel
Copy link
Contributor

iocanel commented Jul 2, 2020

This has been fixed along with #10332. We can close now.

@iocanel iocanel closed this as completed Jul 2, 2020
@gsmet gsmet added this to the 1.7.0 - master milestone Jul 17, 2020
iocanel added a commit to iocanel/quarkus that referenced this issue Jul 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants