-
Notifications
You must be signed in to change notification settings - Fork 23
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
Group pods under aerogear-metrics application and use deploy instead of deploymentConfig #34
base: master
Are you sure you want to change the base?
Conversation
@matzew would appreciated your review on this given you're also working on https://issues.jboss.org/browse/AEROGEAR-2012 |
👀 at it now |
While I see that the Here is the change from
However, the apb fails here See the log:
This brings me to the question, why we are still using |
That check in the APB for the pods to be ready is heavily dependent on your system and how quick the pods come up. If it takes a while for your system to pull the image the check could time out or if it's under heavy load the check could timeout, but when you check the project in Openshift the pod could be fine seconds after the APB provision has 'failed'. I've been investigating the CI failures on this APB and it usually seems to be these container checks timing out, I also can't find checks like these in any other APBs and my instinct is to remove them. Without them the APB won't wait for each container to become ready so the whole APB should provision faster, so this is what i'd like to do:
I can't find a similar health endpoint for prometheus however. @philbrookes @david-martin @matzew does this seem reasonable. I'll create an issue as it's not really in the scope of this PR. |
@StevenTobin Readiness probes for health endpoints sounds reasonable. |
@StevenTobin I also see that the Perhaps we wanna use something like this: |
+1 on moving this larger discussion to a new issue/thread |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is related, but after deploying metrics with this PR I don't see it in my list of services after running |
@secondsun I think this will address it disappearing from the cli |
@secondsun did this work for you before? Because i think it shouldn't work work. |
Bump! I just noticed that DeploymentConfigs were being used, and was considering changing to Deployments. Thankfully I noticed this PR first! |
This PR groups all the metrics pods under the application label aerogear-metrics and switches to the use of k8s deploy resources instead of Openshift deploymentConfigs.
Related issues: #30 & #31