Skip to content

Conversation

@SNiemann15
Copy link
Contributor

@SNiemann15 SNiemann15 commented Feb 18, 2022

@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 18, 2022
@netlify
Copy link

netlify bot commented Feb 18, 2022

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 757b305

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/621ce88bf788c50007f38ca9

😎 Browse the preview: https://deploy-preview-42121--osdocs.netlify.app

Comment on lines -72 to -95

Choose a reason for hiding this comment

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

Need this to populate the DB with the sample data.

Copy link
Contributor

Choose a reason for hiding this comment

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

with the new app, there is no need for populating the DB with the sample data, the application should initialize the DB o its own.

@johusta
Copy link

johusta commented Feb 18, 2022

  1. Under Deploying a PostgreSQL Operator - you cannot use oc apply -n my-petclinic -f - << EOD you have to use oc apply -f - << EOD being the installs in this yaml happen in different namespaces. You will want to decide if you want to be consistent for the rest of the steps or specify the -n my-petclinic to be consistent with x86.
  2. Creating a PostgreSQL database - so we cannot use the same format at x86. the postgresql crunchy operator they use has SBO support built in. We have to put the SBO information as annotations in the metadata section of the postgresql deployment to allow us to use our postgresql operator to work with SBO. I can send you my working yaml or use the old yaml.
  3. Deploying the spring pet clinic app. I tried to use what is specified but I get the following during creation as it relates to the section Service Stanza.
deployment.apps/spring-petclinic created error: error parsing STDIN: error converting YAML to JSON: yaml: line 5: did not find expected key  

I will send you the correct format plus protocol needs to be TCP. I cannot get it format properly here.

This is the example output after the app creation

NAME                                READY   STATUS             RESTARTS        AGE
sampledatabase-9688b5f6d-565fr      1/1     Running            0               47m
spring-petclinic-594df7977b-khpms   0/1     CrashLoopBackOff   13 (104s ago)   44m

I was able to create the Service Binding however the springpet clinic app continues to fail with the following error:

Caused by: org.springframework.jdbc.datasource.init.ScriptStatementFailedException: Failed to execute SQL script statement #1 of URL [jar:file:/tmp/petclinic.jar!/BOOT-INF/classes!/db/postgres/schema.sql]: CREATE TABLE IF NOT EXISTS vets ( id INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, first_name TEXT, last_name TEXT ); nested exception is org.postgresql.util.PSQLException: ERROR: syntax error at or near "GENERATED"
  Position: 42

So new app is not working for me.

@Srivaralakshmi
Copy link
Contributor

Srivaralakshmi commented Feb 21, 2022

@SNiemann15: PTAL at this comment in my PR. I prefer us to maintain consistency in our QSG content and writing style. Hence sending my pov for your consideration. Please let me know WDYT. Thanks!

@Srivaralakshmi
Copy link
Contributor

@SNiemann15 Just a small observation. The QSG for x86 does not contain this prerequisite anymore: "You have installed PostgreSQL psql CLI." I did mention it in the list of changes over slack explicitly (the 1st one in the list). PTAL and do the needful for the SBO QSG for IBM Z and Power.

Comment on lines 48 to 50
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
labels:
app: spring-petclinic
name: spring-petclinic
labels:
app: spring-petclinic
name: spring-petclinic

you have a bad indentation here

@SNiemann15
Copy link
Contributor Author

@SNiemann15 Just a small observation. The QSG for x86 does not contain this prerequisite anymore: "You have installed PostgreSQL psql CLI." I did mention it in the list of changes over slack explicitly (the 1st one in the list). PTAL and do the needful for the SBO QSG for IBM Z and Power.

@Srivaralakshmi I know I left it there for the moment because @johusta wanted to verify if the prereq is still needed for P/Z or not. Once I get confirmation that it's not needed anymore I will remove.

@pmacik
Copy link
Contributor

pmacik commented Feb 21, 2022

@johusta Looks like the dev4dev postgresql uses a different dialect for SQL, let me try to fix it - once I do, I'll re-push the updated image to the same URI quay.io/service-binding/spring-petclinic:latest so it should not affect the content of the docs

Copy link
Contributor

Choose a reason for hiding this comment

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

this should not change - that introduces a wrong indentation and the apply fails

@pmacik
Copy link
Contributor

pmacik commented Feb 21, 2022

@johusta Looks the old PG image version (9.6) might be the problem with the new app. Could you you try to update the DB image (https://github.com/openshift/openshift-docs/pull/42121/files#diff-807b32351555f711f63cdde21d3ef6702d04bed8c5af4b9f1b79f703129828b5R30) from registry.redhat.io/rhel8/postgresql-96:latest (which is deprecated anyway ) to registry.redhat.io/rhel8/postgresql-13:latest ?

@johusta
Copy link

johusta commented Feb 21, 2022

Step 4. Setup port forwarding...... in the port forward string change spring-petclinic-rest to spring-petclinic

@SNiemann15 SNiemann15 force-pushed the sbo101_ibmpz branch 3 times, most recently from ec7e23b to 513ca9e Compare February 22, 2022 15:57
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2022
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2022
@SNiemann15 SNiemann15 force-pushed the sbo101_ibmpz branch 2 times, most recently from 93688bf to c42fa7e Compare February 24, 2022 10:58
Comment on lines 23 to 26
Copy link

@pravin-dsilva pravin-dsilva Feb 24, 2022

Choose a reason for hiding this comment

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

the correct version v1alpha1 is already specified so version: v1beta1 should be removed.

Suggested change
- group: postgresql.dev4devs.com
version: v1beta1
kind: Database <2>
name: sampledatabase
version: v1alpha1
- group: postgresql.dev4devs.com
kind: Database <2>
name: sampledatabase
version: v1alpha1

Copy link

Choose a reason for hiding this comment

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

Good catch Pravin I added comment to the google doc

@SNiemann15 SNiemann15 force-pushed the sbo101_ibmpz branch 3 times, most recently from efd4ee0 to 80f32c9 Compare February 25, 2022 08:38
@johusta
Copy link

johusta commented Feb 25, 2022

Looks Good To Me

@pravin-dsilva
Copy link

lgtm

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.Connecting spring-petclinic to sample database
.Connecting spring-petclinic to a sample database

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
It will take a few minutes until the `CrashLoopBackOff` status is displayed:
It takes a few minutes until the `CrashLoopBackOff` status is displayed:

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
At this stage, the pod fails to start. If you try to interact with the application, it will return errors.
At this stage, the pod fails to start. If you try to interact with the application, it returns errors.

Copy link
Contributor

@rolfedh rolfedh left a comment

Choose a reason for hiding this comment

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

Nice work. I made some minor suggestions.

Copy link
Contributor

@Srivaralakshmi Srivaralakshmi left a comment

Choose a reason for hiding this comment

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

@SNiemann15 Nice work! Just left a couple of suggestions; nitpicks. PTAL. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
. After the operator is installed, list the operator subscriptions in `openshift-operators` namespace:
. After the operator is installed, list the operator subscriptions in the `openshift-operators` namespace:

Comment on lines +75 to +77
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Formatting issue. please check and rectifiy.

@abrennan89 abrennan89 added peer-review-done Signifies that the peer review team has reviewed this PR qe-approved Signifies that QE has signed off on this PR branch/enterprise-4.9 branch/enterprise-4.10 labels Mar 1, 2022
@abrennan89 abrennan89 added this to the Next Release milestone Mar 1, 2022
@abrennan89 abrennan89 added the multi-arch Label for all IBM Power and Z PRs label Mar 1, 2022
@abrennan89 abrennan89 merged commit 6a66323 into openshift:main Mar 1, 2022
@abrennan89
Copy link
Contributor

/cherrypick enterprise-4.9

@abrennan89
Copy link
Contributor

/cherrypick enterprise-4.10

@openshift-cherrypick-robot

@abrennan89: new pull request created: #42564

Details

In response to this:

/cherrypick enterprise-4.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot

@abrennan89: new pull request created: #42565

Details

In response to this:

/cherrypick enterprise-4.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.9 branch/enterprise-4.10 multi-arch Label for all IBM Power and Z PRs peer-review-done Signifies that the peer review team has reviewed this PR qe-approved Signifies that QE has signed off on this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants