Skip to content
This repository was archived by the owner on Oct 29, 2019. It is now read-only.

init: added check for ldap#722

Open
vitoravelino wants to merge 1 commit into
SUSE:masterfrom
vitoravelino:check-ldap-init
Open

init: added check for ldap#722
vitoravelino wants to merge 1 commit into
SUSE:masterfrom
vitoravelino:check-ldap-init

Conversation

@vitoravelino

@vitoravelino vitoravelino commented Feb 1, 2019

Copy link
Copy Markdown
Contributor

Added a new check to verify if the ldap instance is ready before running
velum dashboard in order to avoid a connection error when creating a new
user.

Signed-off-by: Vítor Avelino vavelino@suse.com

bsc#1121064

Comment thread bin/check_ldap.rb
Comment thread bin/check_ldap.rb Outdated
Comment thread bin/check_ldap.rb
Comment thread bin/init Outdated

@flavio flavio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall LGTM, just update the error handling as requested by Miquel.

@vitoravelino

Copy link
Copy Markdown
Contributor Author

I've realized that I added the check to the wrong file. It should be checked in the run file that belongs to the velum-dashboard container and not in the init that belongs to the dashboard initContainer.

mssola
mssola previously approved these changes Feb 7, 2019
Added a new check to verify if the ldap instance is ready before running
velum dashboard in order to avoid a connection error when creating a new
user.

Signed-off-by: Vítor Avelino <vavelino@suse.com>

bsc#1121064

@MalloZup MalloZup left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thx Vito. Lgtm please retrigger the ci and pray, (from famous book eat pray love and retrigger Ci)

@vitoravelino vitoravelino requested a review from flavio March 8, 2019 17:07
@MalloZup

MalloZup commented Mar 11, 2019

Copy link
Copy Markdown
Contributor

@mssola, @flavio can we merge this?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants