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

PBENCH-1153 #3429

Merged
merged 4 commits into from May 24, 2023
Merged

PBENCH-1153 #3429

merged 4 commits into from May 24, 2023

Conversation

riya-17
Copy link
Member

@riya-17 riya-17 commented May 24, 2023

pbench-agent configuration for server

pbench-agent configuration for server
pravins
pravins previously approved these changes May 24, 2023
Copy link
Member

@pravins pravins left a comment

Choose a reason for hiding this comment

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

Configure with Server-1.0

This should be

Configure with Server 1.0 or above

Else looks good.

@riya-17 riya-17 merged commit 5f1d2c0 into distributed-system-analysis:main May 24, 2023
4 checks passed
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

Again, quick iteration and a minimal framework to start is great, but "I have comments". 😆

@@ -0,0 +1,22 @@
### Configuration

**Note**: This Documentation is only required for administrative purposes.
Copy link
Member

Choose a reason for hiding this comment

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

The apparently intended purpose of this document actually spans both agent and server configuration. The distinctions aren't well specified, and ultimately I'm not sure this really helps much.

Deploying a containerized Pbench Agent isn't an "administrative" function.

### Configuration

**Note**: This Documentation is only required for administrative purposes.
By Default the collected results are moved to pbench-server.
Copy link
Member

Choose a reason for hiding this comment

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

When you write "pbench-server" and "pbench-agent", it looks like you're referring to specific commands. Let's stick with "Pbench Agent" and "Pbench Server" to denote the "entities" and avoid that ambiguity. We should also acknowledge (though probably not in much detail here) that one can do a lot with the Pbench Agent command set without ever involving a Pbench Server: pushing datasets to a server is not the primary or only purpose of the Pbench Agent, and "by default" is a bit of a stretch considering you need to use a special command and have correctly configured your security settings to make it work.

Comment on lines +7 to +17
#### Configure with Server < 1.0

For the purpose of defining the required configuration, a pbench-agent configuration file must be created. The config file and the ssh key file must be present in the specified place for the ansible roles to function.

The installation includes a sample configuration file at '/opt/pbench-agent/config/pbench-agent.cfg'. Make a backup of this file, then update the lines marked with # CHANGE ME! comments to suit your setup. Please make sure to make this file accessible to users.

The ssh key pair can be generated with:

ssh-keygen -t rsa

with an empty passphrase. The private key must be made available to users before they can complete the installation of pbench-agent as stated above. The authorized_keys list should include the public key.
Copy link
Member

Choose a reason for hiding this comment

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

I don't find this particularly useful. First off, if you're giving the specific command to generate a key pair, shouldn't we be saying what to do with them? The public key needs to be in the pbench account ~/.ssh/authorized_keys on the Pbench Server, while the private key needs to be in /opt/pbench-agent/id-rsa on each Pbench Agent installation.

There's also the point that this section loosely implies actions on both the Pbench Agent and the Pbench Server, but the reference to "The installation" isn't clear.

I'd also hope we can drop all mention of 0.69 Pbench Server from main, but sadly that would probably be premature as we'll likely continue to support that for at least 0.73.


#### Configure with Server < 1.0

For the purpose of defining the required configuration, a pbench-agent configuration file must be created. The config file and the ssh key file must be present in the specified place for the ansible roles to function.
Copy link
Member

Choose a reason for hiding this comment

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

For review purposes, if nothing else, keeping .md file lines to under 80 columns would be nice. The GitHub view wraps this line, but there's a single line to select and comment on, which can make things awkward. The MarkDown processor doesn't care about hard line wraps, but it can make things easier on lowly humans.

"For the purpose of defining the required configuration" is a very unwieldy sentence. In fact, I'd get rid of the first sentence and clarify the context of the second as something like "To integrate with a 0.69 Pbench Server, a Pbench Agent configuration needs the address of the Pbench Server and a private RSA key trusted by the Pbench Server pbench account." If you want to talk about Ansible installation, we're really into direct RPM installation, whereas I think we're hoping to encourage more people to use the Pbench Agent containers.

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

Successfully merging this pull request may close these issues.

None yet

4 participants