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

AEROGEAR-3075 Group apps into single one #65

Merged
merged 5 commits into from Jun 13, 2018

Conversation

psturc
Copy link
Contributor

@psturc psturc commented Jun 13, 2018

https://issues.jboss.org/browse/AEROGEAR-3075

While putting all services under single group could make the project more clear, it also make an app a bit poorly arranged due to hiding routes to some services (in this case Grafana & Prometheus route):

screen shot 2018-06-13 at 13 24 26

The routes are visible once the row is clicked on (expanded)

screen shot 2018-06-13 at 13 25 13

IMO this is not a big issue, but I'm rather mentioning it.

@psturc
Copy link
Contributor Author

psturc commented Jun 13, 2018

I'd recommend to merge #64 first, because testing APBs on Wendy is pretty unstable.

@aliok
Copy link
Contributor

aliok commented Jun 13, 2018

👁️


- name: delete configmap template file
file: path=/tmp/configmap.yaml state=absent

Copy link
Contributor

Choose a reason for hiding this comment

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

@psturc what's this block doing?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, got it. we were missing the config map on ephemeral installations. Thanks for fixing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is just a copy-pasta of provision-app-metrics.yml file except PV and PVC configuration. It was needed because the tests were failing on Wendy if Postgres was deployed with persistent volume attached to it.
I've only updated it, because some changes from provision-app-metrics.yml were apparently not applied to this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But when I'm thinking about it now, we can probably remove the whole file, because testing on ci-rhos (as proposed in this PR: #64) does not have this issue with PVs anymore.
I'll update this PR once #64 is merged

@aliok
Copy link
Contributor

aliok commented Jun 13, 2018

@psturc when I compare the ephemeral yml with the other yml, I see 2 diffs that could be missed:

Could you please have a look?

screenshot from 2018-06-13 15-51-52

@psturc
Copy link
Contributor Author

psturc commented Jun 13, 2018

@aliok good catch, thanks. I've fixed the wrong labels and removed "ephemeral" role.

Copy link
Contributor

@aliok aliok left a comment

Choose a reason for hiding this comment

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

looks good!

verified locally.

@david-martin
Copy link
Contributor

@psturc I wonder if there's a way to control which route/url shows up as the main one in the overview screen when the 'application' is collapsed.
If we can control it, I'd suggest the Grafana url is shown as thats the only one developers will be directed to in docs.

@david-martin
Copy link
Contributor

@psturc We can set the following annotation on each route

    console.alpha.openshift.io/overview-app-route: 'true'

The makes all the routes show up in the UI

image

There is also a scoring system for deciding which route to show, but in this case showing all routes is probably best.
https://github.com/openshift/origin-web-console/blob/master/app/scripts/services/routes.js#L109-L134

@psturc
Copy link
Contributor Author

psturc commented Jun 13, 2018

@david-martin that's brilliant, thanks for finding that out! Annotations added in d91fef6

@psturc psturc merged commit 42ea855 into aerogearcatalog:master Jun 13, 2018
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.

None yet

3 participants