appsody / stacks

Appsody application stacks. This repo will be archived soon.
https://appsody.dev
Apache License 2.0
89 stars 120 forks source link

Implement service monitoring support for java-openliberty and add timeoutSeconds for readinessProbe #443

Open Kamran64 opened 4 years ago

Kamran64 commented 4 years ago

Following https://github.com/appsody/appsody-operator/pull/125, the Appsody operator supports creating service monitor if there is a spec.monitoring entry in the app-deploy.yaml file. The only requirement to utilise this is a label field, like so:

apiVersion: appsody.dev/v1beta1
kind: AppsodyApplication
metadata:
  name: example-appsodyapplication
spec:
  ....
  monitoring:
    endpoints:
    - basicAuth:
        password:
          key: password
          name: pass
        username:
          key: username
          name: user
      interval: 5s
      tlsConfig:
        insecureSkipVerify: true
    labels:
      k8s-app: APPSODY_PROJECT_NAME

We introduced this functionality for Python Flask, Node.js-Express, and Node.js-functions but it would be nice to see this for the Open Liberty stack also.

Currently, we also see a Knative error if a stack uses the readinessProbe and timeoutSeconds is not defined.

Describe the solution you'd like Add a new field (monitoring) to the app-deploy file underneath spec. The java-microprofile stack has authenticated endpoints and therefore requires a basicAuth field also.

Also add a new field, timeoutSeconds under the readinessProbe and assign it an appropriate value e.g. 1.

Additional context Example PR to implement service monitor support into a stack: https://github.com/appsody/stacks/pull/419/files

Note: The PR above is for a stack without an authenticated endpoint, see: https://github.com/appsody/appsody-operator/pull/125 for a more suitable approach for microprofile.

scottkurz commented 4 years ago

Though this issue hasn't been solved in the java-microprofile stack, relabeling to the java-openliberty stack, which is where we would prioritize a solution if we decide to fix this.

Kamran64 commented 4 years ago

Thanks @scottkurz - I've updated the issue title and description :)

scottkurz commented 4 years ago

@arthurdm correct me if I'm wrong and you see a need here... but I think this is a moot point now that we've switched to the Open Liberty operator. Closing.

awisniew90 commented 4 years ago

I think we can resolve this issue with doc showing how to add a service monitor to the app-config.yaml. We actually show how to do this already here: https://developer.ibm.com/tutorials/configure-an-observable-microservice-with-appsody-openshift-open-liberty/

My concern with adding this to the default app-deploy.yaml is that it requires SSL to be enabled in the deployment which we are choosing not to do by default. Instead, we are adding doc in our readme on how to enable the HTTPS port and configure user credentials via ENV vars from a Secret. (#750 )

Other than the above doc, we can also add a section to our readme on how to enable a Service Monitor after SSL is enabled.