Skip to content
This repository has been archived by the owner on Apr 21, 2023. It is now read-only.

aiming to close spot-23 #24

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

Conversation

natedogs911
Copy link
Contributor

@natedogs911 natedogs911 commented Feb 24, 2017

QA and Review and advise on any changes please,
This will require some doc changes.

known issues:

  • spot-nfdump must be fixed upstream and version number changed in spot-ingest/install.sh

TBD:

  • error handling needs to happen in all scripts. need an error handling function
  • would like to keep installs down to 2 scripts, looking for feedback on better ways to do this
  • need a better solution then spot-ml/post_build.sh to install in /opt/spot/bin
  • add launch scripts for each component in /opt/spot/bin (i.e. start ingest, run OA etc)

Discussion:

should the assumption be made that all components are being landed on the same server?
This would greatly simplify things as in spot-setup you could just run a script to source each components scripts. feedback would be great.

EDIT: really there is only the one known issue, nfdump is in the spot-nfdump repo that i'm pulling in and must be fixed there.

@natedogs911 natedogs911 changed the title Spot 23 aiming to close spot-23 Feb 24, 2017
@natedogs911
Copy link
Contributor Author

aims to close the #spot-23 jira with pr #24

@rabarona
Copy link

rabarona commented Feb 24, 2017

Spot-23: https://issues.apache.org/jira/browse/SPOT-23
(for some reason # is not linking this PR with the Jira issue).

Hey, @natedogs911 would you paste the discussion part in the JIRA issue?
Also, are the known issues something happening in the code you are pushing or it's exactly what your code is fixing?

@natedogs911
Copy link
Contributor Author

I edited my comments, nfdump is the known issue, the rest are things that i want to improve/add

Copy link

@rabarona rabarona left a comment

Choose a reason for hiding this comment

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

If there are no particular reasons to keep separated scripts in ML and Ingest, I would put the installation all together in a single script.
If the scripts get too long, then I would recommend writing different pieces but then have only one one script to call each functionality ( build, post build, etc.).

pip install -r requirements.txt
fi

log_cmd "dependencies satisfied, please run ./build.sh to complete setup"

Choose a reason for hiding this comment

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

spot-ingest install.sh is asking to run build.sh but there is no spot-ingest/build.sh, is this message something should be here or build.sh is missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally i did have a build.sh, but as i refactored things it became redundant.
I simply forgot to update this message.


1. run `sudo ./install.sh`
2. run `./build.sh`
3. run `sudo ./post_build.sh`

Choose a reason for hiding this comment

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

I vote to put these three together in a single script and just print the "state" of the installation like "SBT installation, SBT assembly and JAR installation".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason for separate scripts is because you need root for the installation and post build steps, if the consensus is that root should be used for all of these steps then i agree.

Choose a reason for hiding this comment

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

What about using sudo -H -E? I'm not even sure that's the syntax but I remember using something like that to elevate permissions but still keep using current user.

## Install

1. run `sudo ./install.sh`
2. run `hdfs_setup.sh`

Choose a reason for hiding this comment

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

Wondering if it's possible to automatize the step of moving spot.conf to /etc/ folder in Spot Installation Documentation, step 3.4.

Also, about your point for discussion, this PR assumes then that everything including spot-ml, spot-ingest and spot-oa is going to be landed in a single node, am I correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, spot.conf could be added very easily.
However i was thinking i might do move that to /opt/spot/spot.conf instead. this requires changing each components file path which should be fairly simple.

In regards to the discussion,
It would be simple to have a install-all.sh script that ran everything,

#!/bin/bash

source ./install.sh
source ../spot-ingest/install.sh
source ../spot-ml/install.sh
source ../spot-oa/install.sh
and then a user can make the choice if they are landing individual components elsewhere.

Choose a reason for hiding this comment

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

In case users want to land in different nodes, then they would need to run separate scripts but if they do in a single node, then just run install-all.sh, is that what you are saying?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's correct

EverLoSa pushed a commit to EverLoSa/incubator-spot that referenced this pull request Mar 13, 2017
@natedogs911
Copy link
Contributor Author

Ready for review and merge...

Copy link

@rabarona rabarona left a comment

Choose a reason for hiding this comment

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

I have a few questions:

  1. About documentation changes, are you talking about Spot web page changes? Or more README changes?
  2. When you say we need error handling, are you saying we need a "standard" way to print errors? Or you are talking about actually handling errors? Are any errors being handled in this version?
  3. I don't see spot.conf scp, are users still required to copy manually? It would be really great if we can make this automatic.
  4. I don't see install-all.sh either, so the decision was not to have a script like that?
  5. Finally, could you please write a quick description of how the installation process is going to be after this PR?

Thanks.

@raypanduro
Copy link
Contributor

+1

@natedogs911
Copy link
Contributor Author

RE: @rabarona

  1. README changes, Yes the web needs to be updated.

  2. a little bit of both, i'd like to catch errors and notify the user "installation failed at line 22, with 'foo'". it's fairly trivial and can easily be added with more iterations.

  3. I didn't change the workflow for the conf file, although i can add something like this:

# copy spot.conf if [[ -f ./spot.conf ]]; then cp ./spot.conf /etc/spot.conf log_cmd "Copied spot.conf into /etc/" else log_cmd "Please run \cp ./spot.conf.template ./spot.conf` and customize before running ./install.sh"
exit 1
fi`

and change the spot.conf file to spot.conf.template

  1. I think a more elegant solution is needed then just sourcing each individual installation script. this is somewhat related to my answer to question 5.

  2. Should the assumption be made that all components will be run from the same node? if so then,
    (assuming i make the above change)

a. from the ./spot-setup run cp spot.conf.template spot.conf and edit the configurations for your specific cluster

b. run sudo ./install.sh

c. per ./spot-setup/README.md run ./hdfs_setup.sh

d. from ./spot-ingest run sudo ./install.sh

e. while in ./spot-ml run sudo ./install.sh

f. while in ./spot-oa run sudo ./install.sh

If you want to run a particular component such as spot-ingest on another node, simply copy /etc/spot.conf to that node along with the spot-ingest folder and run the ./install.sh for that component.

@natedogs911
Copy link
Contributor Author

how about to simplify the 6 step process outlined above we can add an --install-all option to the spot-setup/install.sh script which executes the script for each component?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants