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

Fix rkt mounts and add arbitrary volume mounting #1812

Merged
merged 10 commits into from
Oct 25, 2016
Merged

Conversation

schmichael
Copy link
Member

Just like #1767 did for the docker driver.


// rktVolumesConfigOption is the key for enabling the use of custom
// bind volumes.
rktVolumesConfigOption = "rkt.volumes.enabled"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this documented on the website? Plus the docker option

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

cmdArgs = append(cmdArgs, fmt.Sprintf("--mount=volume=%s,target=%s", task.Name, ctx.AllocDir.SharedDir))

// Mount /alloc
cmdArgs = append(cmdArgs, fmt.Sprintf("--volume=%salloc,kind=host,source=%s", task.Name, ctx.AllocDir.SharedDir))
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we should be using file path.Join and using the constants

Copy link
Member Author

Choose a reason for hiding this comment

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

To generate the source=%s path? Won't that just be reimplementing what's in ctx.AllocDir.SharedDir?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed the unique volume names.

@@ -9,6 +9,7 @@ IMPROVEMENTS:
* client: Introduce a `secrets/` directory to tasks where sensitive data can
be written [GH-1681]
* driver: Export `NOMAD_JOB_NAME` environment variable [GH-1804]
* driver/rkt: Support rkt volumes (rkt >= 1.0.0 required) [GH-1812]
Copy link
Contributor

Choose a reason for hiding this comment

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

Lexicographical ordering, rkt after docker


// Mount arbitrary volumes if enabled
if len(driverConfig.Volumes) > 0 {
if enabled := d.config.ReadBoolDefault(rktVolumesConfigOption, false); !enabled {
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to default true

## Task Directories
* `volumes` - (Optional) A list of `host_path:container_path` strings to bind
host paths to container paths. Can only be run on clients with the
`rkt.volumes.enabled` option set to true.
Copy link
Contributor

Choose a reason for hiding this comment

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

The wording should slightly change based on changing the default

@@ -141,7 +149,7 @@ func (d *RktDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, e
return false, nil
}

outBytes, err := exec.Command("rkt", "version").Output()
outBytes, err := exec.Command(rktCmd, "version").Output()
Copy link
Contributor

Choose a reason for hiding this comment

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

Add rktVolumesConfigOption to the attributes during fingerprinting

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants