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

PNDA-4780 CDH: orchestrate-pnda-knox fails on centos in openstack #582

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

janselva
Copy link
Contributor

Issue: Knox deployment failed in CDH

Solution:
Added Hadoop manager based condition to handle both CDH and HDP.

@trsmith2 trsmith2 self-requested a review July 12, 2018 15:10
@trsmith2 trsmith2 self-assigned this Jul 12, 2018
- '"/var/log/pnda/hue/*.log"'
hadoop_service_name:
Copy link
Member

Choose a reason for hiding this comment

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

It's unusual to specify things in the Hadoop section but only specify them for one Hadoop version. Please look at how these pillar sections are used elsewhere in the code.

If you need a generic service name abstractor for Hadoop, that's fine - but please use it consistently, I still see e.g. 'oozie01' in the code even though this abstraction has been placed in the pillar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

working for the New fix will update once it's completed.

@@ -150,6 +150,7 @@ def cloudera_get_hosts_by_hadoop_role(service, role_type):
password = hadoop_manager_password()
endpoint = hadoop_manager_ip() + ':7180'
cluster = cluster_name()
service = __salt__['pillar.get']('hadoop_service_name:'+service)
Copy link
Member

Choose a reason for hiding this comment

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

Please see above comment about pillar usage

{% set mr2_history_server_host = salt['pnda.get_hosts_by_hadoop_role']('MAPREDUCE2', 'HISTORYSERVER')[0] %}
{% set spark_history_server_host = salt['pnda.get_hosts_by_hadoop_role']('SPARK', 'SPARK_JOBHISTORYSERVER')[0] %}
{% set ambari_server_host = salt['pnda.get_hosts_for_role']('hadoop_manager')[0] %}
{% set hadoop_manager_host = salt['pnda.get_hosts_for_role']('hadoop_manager')[0] %}
Copy link
Member

Choose a reason for hiding this comment

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

This change is incorrect. This isn't a generic Hadoop manager, it's Ambari. The knox config for Ambari will not work for anything else.

@@ -26,6 +23,23 @@
{% set opentsdb_version = pillar['opentsdb']['version'] %}
{% set helper_directory = knox_home_directory + '/helper' %}

{%- if grains['hadoop.distro'] == 'CDH' -%}
Copy link
Member

Choose a reason for hiding this comment

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

Please see above comment about Hadoop usage and avoid replication like this, it leads to bugs (see DRY)

Copy link
Member

@trsmith2 trsmith2 left a comment

Choose a reason for hiding this comment

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

Please see comments inline

@janselva
Copy link
Contributor Author

janselva commented Aug 7, 2018

Updated the changes,
tested in HDP ,CDH standard cluster.

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

Successfully merging this pull request may close these issues.

2 participants