You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by justinleet <gi...@git.apache.org> on 2016/08/22 12:42:34 UTC

[GitHub] incubator-metron pull request #222: METRON-385 Create Ambari Service Definit...

GitHub user justinleet opened a pull request:

    https://github.com/apache/incubator-metron/pull/222

    METRON-385 Create Ambari Service Definition for Indexing

    https://issues.apache.org/jira/browse/METRON-385
    
    Opening this PR for community review and feedback.
    
    Prereq:
    * Ambari 2.4
    
    Testing this PR:
    * The RPMs in metron-deployment should be built.  They should either be mounted or copied to /localrepo in the node to be used as the indexing master node.
    * Copy or mount the services and stack contents into the appropriate places on the ambari-server node.  For example:
    ** incubator-metron/metron-deployment/packaging/ambari/src/main/resources/stacks/HDP/2.4/services/INDEXING -> /var/lib/ambari-server/resources/stacks/HDP/2.4/services/INDEXING
    **incubator-metron/metron-deployment/packaging/ambari/src/main/resources/common-services/INDEXING -> /var/lib/ambari-server/resources/common-services/INDEXING
    * Install the master to a node with the Kafka Broker.
    
    Limitations / Notes
    * Service advisor for ensuring that the service is colocated appropriately doesn't appear to work. It's currently in the PR as-is.  It can easily be removed if we want to leave it out for now.
    * Because the RPMs are currently not published to a yum repo, it's necessary to do the copy / mount step. Assuming the RPMs find a more permanent home, the repolist for the used stack should be updated and preferred.
    * Elasticsearch.properties had a couple properties added to reflect what was in the Ansible scripts, because the current file was missing properties necessary to run the script outside of that context.
    * Ambari currently provides no hooks for performing actions when a service is deleted.  To ensure cleanup please run
    
    ```
    #!/bin/bash
    
    # kafka
    /usr/hdp/current/kafka-broker/bin/kafka-topics.sh \
            --zookeeper $ZK_QUORUM \
            --delete \
            --topic indexing
    done
    
    # storm kill
    storm kill indexing
    
    # yum delete
    yum -y erase metron-indexing metron-common
    
    # final file delete
    rm -rf /usr/metron
    ```


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/justinleet/incubator-metron ambari-indexing

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-metron/pull/222.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #222
    
----
commit 3172d9137b05f3f4c271f2a121fa4223606c8221
Author: justinjleet <ju...@gmail.com>
Date:   2016-08-17T12:14:36Z

    WIP Indexing Service

commit d147e24097a76032b64f705456e6f3cfc335bff3
Author: justinjleet <ju...@gmail.com>
Date:   2016-08-17T12:50:06Z

    Metron Indexing Service in Ambari

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron issue #222: METRON-385 Create Ambari Service Definition for...

Posted by james-sirota <gi...@git.apache.org>.
Github user james-sirota commented on the issue:

    https://github.com/apache/incubator-metron/pull/222
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron pull request #222: METRON-385 Create Ambari Service Definit...

Posted by dlyle65535 <gi...@git.apache.org>.
Github user dlyle65535 commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/222#discussion_r75869071
  
    --- Diff: metron-platform/metron-elasticsearch/src/main/config/elasticsearch.properties ---
    @@ -57,10 +57,13 @@ org.apache.metron.metrics.TelemetryParserBolt.fails=true
     
     bolt.hdfs.batch.size=5000
     bolt.hdfs.field.delimiter=|
    +bolt.hdfs.rotation.policy=org.apache.storm.hdfs.bolt.rotation.TimedRotationPolicy
    +bolt.hdfs.rotation.policy.units=DAYS
    +bolt.hdfs.rotation.policy.count=1
    +
     bolt.hdfs.file.rotation.size.in.mb=5
    --- End diff --
    
    Oh, I get it. Yeah, we'll likely need to expose it as configs, but for now it makes sense.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron pull request #222: METRON-385 Create Ambari Service Definit...

Posted by dlyle65535 <gi...@git.apache.org>.
Github user dlyle65535 commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/222#discussion_r75862174
  
    --- Diff: metron-platform/metron-elasticsearch/src/main/config/elasticsearch.properties ---
    @@ -57,10 +57,13 @@ org.apache.metron.metrics.TelemetryParserBolt.fails=true
     
     bolt.hdfs.batch.size=5000
     bolt.hdfs.field.delimiter=|
    +bolt.hdfs.rotation.policy=org.apache.storm.hdfs.bolt.rotation.TimedRotationPolicy
    +bolt.hdfs.rotation.policy.units=DAYS
    +bolt.hdfs.rotation.policy.count=1
    +
     bolt.hdfs.file.rotation.size.in.mb=5
    --- End diff --
    
    I'm a little confused by this as part of this PR. Can you tell me a bit about it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron pull request #222: METRON-385 Create Ambari Service Definit...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/222#discussion_r77627924
  
    --- Diff: metron-deployment/packaging/ambari/src/main/resources/common-services/INDEXING/0.2.0BETA/service_advisor.py ---
    @@ -0,0 +1,65 @@
    +#!/usr/bin/env ambari-python-wrap
    +"""
    +Licensed to the Apache Software Foundation (ASF) under one
    +or more contributor license agreements.  See the NOTICE file
    +distributed with this work for additional information
    +regarding copyright ownership.  The ASF licenses this file
    +to you under the Apache License, Version 2.0 (the
    +"License"); you may not use this file except in compliance
    +with the License.  You may obtain a copy of the License at
    +    http://www.apache.org/licenses/LICENSE-2.0
    +Unless required by applicable law or agreed to in writing, software
    +distributed under the License is distributed on an "AS IS" BASIS,
    +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +See the License for the specific language governing permissions and
    +limitations under the License.
    +"""
    +import imp
    +import os
    +import traceback
    +
    +SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
    +STACKS_DIR = os.path.join(SCRIPT_DIR, '../../../stacks/')
    +PARENT_FILE = os.path.join(STACKS_DIR, 'service_advisor.py')
    +
    +try:
    +    with open(PARENT_FILE, 'rb') as fp:
    +        service_advisor = imp.load_module('service_advisor', fp, PARENT_FILE, ('.py', 'rb', imp.PY_SOURCE))
    +except Exception as e:
    +    traceback.print_exc()
    +    print("Failed to load parent service_advisor file '{}'".format(PARENT_FILE))
    +
    +
    +class INDEXING020BETAServiceAdvisor(service_advisor.ServiceAdvisor):
    --- End diff --
    
    The Metron version is embedded in this class name.  Also, the path for the Indexing `metron-deployment/packaging/ambari/src/main/resources/common-services/INDEXING/0.2.0BETA/...` includes the Metron version.  
    
    How will we handle this when we bump Metron versions?  Is there no better way to handle this?  I know we have to play within the sandbox provided by Ambari.  Is this an Ambari-imposed pain point?
    
    Everything else looks great BTW.  Very easy to read code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron pull request #222: METRON-385 Create Ambari Service Definit...

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/222#discussion_r77641194
  
    --- Diff: metron-deployment/packaging/ambari/src/main/resources/common-services/INDEXING/0.2.0BETA/service_advisor.py ---
    @@ -0,0 +1,65 @@
    +#!/usr/bin/env ambari-python-wrap
    +"""
    +Licensed to the Apache Software Foundation (ASF) under one
    +or more contributor license agreements.  See the NOTICE file
    +distributed with this work for additional information
    +regarding copyright ownership.  The ASF licenses this file
    +to you under the Apache License, Version 2.0 (the
    +"License"); you may not use this file except in compliance
    +with the License.  You may obtain a copy of the License at
    +    http://www.apache.org/licenses/LICENSE-2.0
    +Unless required by applicable law or agreed to in writing, software
    +distributed under the License is distributed on an "AS IS" BASIS,
    +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +See the License for the specific language governing permissions and
    +limitations under the License.
    +"""
    +import imp
    +import os
    +import traceback
    +
    +SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
    +STACKS_DIR = os.path.join(SCRIPT_DIR, '../../../stacks/')
    +PARENT_FILE = os.path.join(STACKS_DIR, 'service_advisor.py')
    +
    +try:
    +    with open(PARENT_FILE, 'rb') as fp:
    +        service_advisor = imp.load_module('service_advisor', fp, PARENT_FILE, ('.py', 'rb', imp.PY_SOURCE))
    +except Exception as e:
    +    traceback.print_exc()
    +    print("Failed to load parent service_advisor file '{}'".format(PARENT_FILE))
    +
    +
    +class INDEXING020BETAServiceAdvisor(service_advisor.ServiceAdvisor):
    --- End diff --
    
    It's an Ambari imposed pain point. From what I can tell, it just looks for that class name via magic. If this actually existed in it's own stack, we could use the Stack advisor. Service advisor is more for adding custom services to another stack, so this is probably closer to what we can do until everything coalesces a little more nicely.
    
    The better way is probably to have this handled by an Ambari mpack (which bundles everything up on it's own), but I'm not sure how nicely Service and Stack advisors play, particularly when an mpack is installed alongside an existing stack.  It's something I don't know enough about to say.
    
    Relevant portion of the [Ambari wiki](https://cwiki.apache.org/confluence/display/AMBARI/How-To+Define+Stacks+and+Services#How-ToDefineStacksandServices-ServiceAdvisor):
    
    
    "Unlike the Stack-advisor scripts, the service-advisor scripts do not automatically extend the parent service's service-advisor scripts. The service-advisor script needs to explicitly extend their parent's service service-advisor script."
    
    I wouldn't be opposed to dropping the service-advisor for now (especially given the note in the original description).  Especially since I'd like to drop this method anyway in the future, assuming that's feasible.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron pull request #222: METRON-385 Create Ambari Service Definit...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-metron/pull/222


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron issue #222: METRON-385 Create Ambari Service Definition for...

Posted by dlyle65535 <gi...@git.apache.org>.
Github user dlyle65535 commented on the issue:

    https://github.com/apache/incubator-metron/pull/222
  
    +1 looks good!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron issue #222: METRON-385 Create Ambari Service Definition for...

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/incubator-metron/pull/222
  
    One issue discovered, which is that, to make things compatible with earlier Kafka versions that don't handle topics already existing that functionality was dropped. This PR needs to be updated to handle restarts properly without that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron issue #222: METRON-385 Create Ambari Service Definition for...

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/incubator-metron/pull/222
  
    Added a change to ensure that this works with centos 6's default Python version (2.6.x)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron pull request #222: METRON-385 Create Ambari Service Definit...

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/222#discussion_r75867968
  
    --- Diff: metron-platform/metron-elasticsearch/src/main/config/elasticsearch.properties ---
    @@ -57,10 +57,13 @@ org.apache.metron.metrics.TelemetryParserBolt.fails=true
     
     bolt.hdfs.batch.size=5000
     bolt.hdfs.field.delimiter=|
    +bolt.hdfs.rotation.policy=org.apache.storm.hdfs.bolt.rotation.TimedRotationPolicy
    +bolt.hdfs.rotation.policy.units=DAYS
    +bolt.hdfs.rotation.policy.count=1
    +
     bolt.hdfs.file.rotation.size.in.mb=5
    --- End diff --
    
    Those lines are actually added by Ansible in the original path (topologies.yml).  They don't exist with any properties in elasticsearch.properties (which gets bundled up as part of the RPM build).  Because the properties don't exist, the topology will fail.
    
    I just used what Ansible defined as the configs, but they're easy to change if we want to (or if there's some difference between environments I missed).
    
    Otherwise if we want to leave them out of elasticsearch.properties, we'll probably want to do something a little more clever (e.g. make elasticsearch.properties a config file handled through Ambari and use that instead of what the RPM has).  I'm unsure if that file gets used in other manners in other deploys, so I was hesitant to make that change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron issue #222: METRON-385 Create Ambari Service Definition for...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:

    https://github.com/apache/incubator-metron/pull/222
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron pull request #222: METRON-385 Create Ambari Service Definit...

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/222#discussion_r78366393
  
    --- Diff: metron-deployment/packaging/ambari/src/main/resources/common-services/INDEXING/0.2.0BETA/service_advisor.py ---
    @@ -0,0 +1,65 @@
    +#!/usr/bin/env ambari-python-wrap
    +"""
    +Licensed to the Apache Software Foundation (ASF) under one
    +or more contributor license agreements.  See the NOTICE file
    +distributed with this work for additional information
    +regarding copyright ownership.  The ASF licenses this file
    +to you under the Apache License, Version 2.0 (the
    +"License"); you may not use this file except in compliance
    +with the License.  You may obtain a copy of the License at
    +    http://www.apache.org/licenses/LICENSE-2.0
    +Unless required by applicable law or agreed to in writing, software
    +distributed under the License is distributed on an "AS IS" BASIS,
    +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +See the License for the specific language governing permissions and
    +limitations under the License.
    +"""
    +import imp
    +import os
    +import traceback
    +
    +SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
    +STACKS_DIR = os.path.join(SCRIPT_DIR, '../../../stacks/')
    +PARENT_FILE = os.path.join(STACKS_DIR, 'service_advisor.py')
    +
    +try:
    +    with open(PARENT_FILE, 'rb') as fp:
    +        service_advisor = imp.load_module('service_advisor', fp, PARENT_FILE, ('.py', 'rb', imp.PY_SOURCE))
    +except Exception as e:
    +    traceback.print_exc()
    +    print("Failed to load parent service_advisor file '{}'".format(PARENT_FILE))
    +
    +
    +class INDEXING020BETAServiceAdvisor(service_advisor.ServiceAdvisor):
    --- End diff --
    
    Removed the service advisor and updated PR


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-metron pull request #222: METRON-385 Create Ambari Service Definit...

Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/222#discussion_r77647135
  
    --- Diff: metron-deployment/packaging/ambari/src/main/resources/common-services/INDEXING/0.2.0BETA/service_advisor.py ---
    @@ -0,0 +1,65 @@
    +#!/usr/bin/env ambari-python-wrap
    +"""
    +Licensed to the Apache Software Foundation (ASF) under one
    +or more contributor license agreements.  See the NOTICE file
    +distributed with this work for additional information
    +regarding copyright ownership.  The ASF licenses this file
    +to you under the Apache License, Version 2.0 (the
    +"License"); you may not use this file except in compliance
    +with the License.  You may obtain a copy of the License at
    +    http://www.apache.org/licenses/LICENSE-2.0
    +Unless required by applicable law or agreed to in writing, software
    +distributed under the License is distributed on an "AS IS" BASIS,
    +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +See the License for the specific language governing permissions and
    +limitations under the License.
    +"""
    +import imp
    +import os
    +import traceback
    +
    +SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
    +STACKS_DIR = os.path.join(SCRIPT_DIR, '../../../stacks/')
    +PARENT_FILE = os.path.join(STACKS_DIR, 'service_advisor.py')
    +
    +try:
    +    with open(PARENT_FILE, 'rb') as fp:
    +        service_advisor = imp.load_module('service_advisor', fp, PARENT_FILE, ('.py', 'rb', imp.PY_SOURCE))
    +except Exception as e:
    +    traceback.print_exc()
    +    print("Failed to load parent service_advisor file '{}'".format(PARENT_FILE))
    +
    +
    +class INDEXING020BETAServiceAdvisor(service_advisor.ServiceAdvisor):
    --- End diff --
    
    If the Service Advisor can be dropped and doesn't affect the core functioning of your PR, then it would be nice to take it out. Less code is more.
    
    It sounds like you have a go-forward plan to resolve the 'version embedded in the path' issue with Ambari mpacks.  I'm good with that plan.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---