Skip to content

Conversation

@AS186170
Copy link

@AS186170 AS186170 commented Apr 6, 2017

PR for all MapR docker containers
Please review and give feedback @ArturGajowy @akshatnair @petroav @sanjay990

singhash and others added 7 commits March 30, 2017 11:13
This image contains installation of all mapr related softwares and there
dependencies
This contaner is enabled with security on both MapR cluster and Hive
The changes includes a new function expose_mapr and the shell script to
wait for MapR , also this changes will enable tests to switch user when
the container is not MapR.
Adding missing sshd start in Docker-base
This changes will make maprcli wait for MapR ticket in wardenTracker
@AS186170 AS186170 requested a review from ArturGajowy April 6, 2017 17:25
Copy link
Contributor

@ArturGajowy ArturGajowy left a comment

Choose a reason for hiding this comment

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

Partial review done.

# CONFIGURE SSH
&& chkconfig sshd on \
&& grep -rl '#Port 22' /etc/ssh/sshd_config | xargs sed -i 's/#Port 22/Port 22/g' \
&& service sshd start \
Copy link
Contributor

Choose a reason for hiding this comment

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

why would that be needed? This is during image build, the newly started container will have sshd down anyway (except for cases when supervisor.d will spin it up)

ADD files/maprtech.repo /etc/yum.repos.d/maprtech.repo
COPY files/id_rsa.pub /root/
RUN yum update -y \
# ... GET MapRGPG KEY
Copy link
Contributor

Choose a reason for hiding this comment

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

missing space

&& rpm --import http://package.mapr.com/releases/pub/maprgpg.key \

# INSTALL UTILITY SOFTWARE
&& yum install -y iputils vim openssh-server openssh-clients sudo lsof \
Copy link
Contributor

Choose a reason for hiding this comment

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

that's not needed, until proven otherwise

Copy link
Author

Choose a reason for hiding this comment

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

apart from vim all others are required

&& yum install -y iputils vim openssh-server openssh-clients sudo lsof \
# CONFIGURE SSH
&& chkconfig sshd on \
&& grep -rl '#Port 22' /etc/ssh/sshd_config | xargs sed -i 's/#Port 22/Port 22/g' \
Copy link
Contributor

Choose a reason for hiding this comment

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

why is that needed? The other containers expose sshd as well and they don't seem to be sed-ing the 22 Port in config anywhere AFAIR?

Copy link
Author

Choose a reason for hiding this comment

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

Its not working without that , other containers will all have same problems when some one will ssh hadoop-master from outside.

&& grep -rl '#Port 22' /etc/ssh/sshd_config | xargs sed -i 's/#Port 22/Port 22/g' \

# INSTALL MAPR
&& yum install -y mapr-fileserver mapr-nfs mapr-nodemanager mapr-cldb \
Copy link
Contributor

Choose a reason for hiding this comment

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

squash into a single yum install invocation

# mapr52-base

Docker image with all MapR related softwares installed and there dependencies.
Copy link
Contributor

Choose a reason for hiding this comment

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

software (it's uncountable)

Copy link
Contributor

Choose a reason for hiding this comment

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

their

@@ -0,0 +1,9 @@
# mapr52-base
Copy link
Contributor

Choose a reason for hiding this comment

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

add badges

supervisorctl start socks-proxy

# CONFIGURE MAPR
/opt/mapr/server/configure.sh -N mycluster -Z localhost -C localhost -HS localhost -no-autostart
Copy link
Contributor

Choose a reason for hiding this comment

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

double space

/opt/mapr/server/configure.sh -R

# WAIT FOR WARDEN TO START ALL THE SERVICES
sh /root/wardenTracker.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

drop sh

@@ -0,0 +1,54 @@
#!/bin/bash

hname=$(hostname)
Copy link
Contributor

Choose a reason for hiding this comment

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

I htink there's a whole lot of comments that haven't been applied for this file

Copy link
Author

@AS186170 AS186170 Apr 11, 2017

Choose a reason for hiding this comment

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

@ArturGajowy can you review Kerberbos MapR which is new,
also the test changes we did I will also look into the old PR comments and fix together

so that the build finishes under the 50 minutes mark
so that the build isn't terminated after long time without output
@ArturGajowy
Copy link
Contributor

ArturGajowy commented Apr 11, 2017 via email

@AS186170
Copy link
Author

@ArturGajowy @petroav
All the review comments are done , and tests are also passing , please complete the final review

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.

4 participants