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

[storewolf] Improve default metadata representation #491

Merged
merged 1 commit into from
Nov 13, 2019
Merged

Conversation

zmrow
Copy link
Contributor

@zmrow zmrow commented Nov 7, 2019

This commit improves how metadata is represented in our defaults.toml.
Previously each metadata entry required a seperate 4 line declaration,
which is noisy and unpleasant. With this change, metadata for each
corresponding setting can be defined together in a single metadata
entry.

The commit adds a single function to translate this new defaults toml
format into a Vec which is processed in storewolf exactly
the same as we did previously.

*Please closely double check my edits of defaults.toml!

Issue #, if available:
Fixes #135

Description of changes:

  • Update defaults.toml to new format
  • Add function to parse new defaults.toml metadata format and convert to Vec

Testing done

  • Datastore on an image created with this PR has identical structure with a prior Thar image.
  • On my dev box I'm able to create a datastore that looks correct
[mrowicki@ip-10-0-60-75 api]$ ../target/debug/storewolf --data-store-base-path /tmp/mrowicki --version 1.0
20:03:09 [ INFO] Storewolf started
20:03:09 [ INFO] Deleting pending settings
20:03:09 [ INFO] Populating datastore at: /tmp/mrowicki
20:03:09 [ INFO] Creating datastore at: /tmp/mrowicki/current/live
20:03:09 [ INFO] Datastore populated

[mrowicki@ip-10-0-60-75 api]$ tree /tmp/mrowicki/
/tmp/mrowicki/
├── current -> v1
├── v1 -> v1.0
├── v1.0 -> v1.0_NLEMxi3UTTT7U6nH
└── v1.0_NLEMxi3UTTT7U6nH
    ├── live
    │   ├── configuration-files
    │   │   ├── chrony-conf
    │   │   │   ├── path
    │   │   │   └── template-path
    │   │   ├── containerd-config-toml
    │   │   │   ├── path
    │   │   │   └── template-path
    │   │   ├── hostname
    │   │   │   ├── path
    │   │   │   └── template-path
    │   │   ├── kubelet-config
    │   │   │   ├── path
    │   │   │   └── template-path
    │   │   ├── kubelet-env
    │   │   │   ├── path
    │   │   │   └── template-path
    │   │   ├── kubelet-kubeconfig
    │   │   │   ├── path
    │   │   │   └── template-path
    │   │   ├── kubernetes-ca-crt
    │   │   │   ├── path
    │   │   │   └── template-path
    │   │   └── updog-toml
    │   │       ├── path
    │   │       └── template-path
    │   ├── services
    │   │   ├── host-containers
    │   │   │   ├── configuration-files
    │   │   │   └── restart-commands
    │   │   ├── hostname
    │   │   │   ├── configuration-files
    │   │   │   └── restart-commands
    │   │   ├── kubernetes
    │   │   │   ├── configuration-files
    │   │   │   └── restart-commands
    │   │   ├── ntp
    │   │   │   ├── configuration-files
    │   │   │   └── restart-commands
    │   │   └── updog
    │   │       ├── configuration-files
    │   │       └── restart-commands
    │   └── settings
    │       ├── host-containers.affected-services
    │       ├── hostname.affected-services
    │       ├── kubernetes
    │       │   ├── cluster-dns-ip.setting-generator
    │       │   ├── max-pods.setting-generator
    │       │   ├── node-ip.setting-generator
    │       │   └── pod-infra-container-image.setting-generator
    │       ├── kubernetes.affected-services
    │       ├── ntp.affected-services
    │       ├── updates
    │       │   └── seed.setting-generator
    │       └── updates.affected-services
    └── pending
        └── settings
            ├── host-containers
            │   ├── admin
            │   │   ├── enabled
            │   │   ├── source
            │   │   └── superpowered
            │   └── control
            │       ├── enabled
            │       ├── source
            │       └── superpowered
            ├── hostname
            ├── ntp
            │   └── time-servers
            ├── timezone
            └── updates
                ├── metadata-base-url
                └── target-base-url

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@zmrow zmrow modified the milestone: Public Preview Nov 7, 2019
workspaces/api/storewolf/defaults.toml Outdated Show resolved Hide resolved
workspaces/api/storewolf/src/main.rs Outdated Show resolved Hide resolved
workspaces/api/storewolf/src/main.rs Outdated Show resolved Hide resolved
workspaces/api/storewolf/src/main.rs Outdated Show resolved Hide resolved
workspaces/api/storewolf/src/main.rs Outdated Show resolved Hide resolved
workspaces/api/storewolf/src/main.rs Show resolved Hide resolved
workspaces/api/storewolf/src/main.rs Outdated Show resolved Hide resolved
workspaces/api/storewolf/src/main.rs Show resolved Hide resolved
#[snafu(display("Internal error: {}", msg))]
Internal { msg: String },

#[snafu(display("Keys can't contain newlines: {}", source))]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this error message just duplicates what's going to be in the source error; it might be better to give a pointer to the input that unexpectedly contained a newline. The path?

workspaces/api/storewolf/src/main.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@etungsten etungsten left a comment

Choose a reason for hiding this comment

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

🌲

@zmrow
Copy link
Contributor Author

zmrow commented Nov 11, 2019

Addressed comments from @tjkirch

workspaces/api/storewolf/src/main.rs Outdated Show resolved Hide resolved
This commit improves how metadata is represented in our defaults.toml.
Previously each metadata entry required a seperate 4 line declaration,
which is noisy and unpleasant. With this change, metadata for each
corresponding setting can be defined together in a single metadata
entry.

The commit adds a single function to translate this new defaults toml
format into a Vec<Metadata> which is processed in storewolf exactly
the same as we did previously.
@zmrow
Copy link
Contributor Author

zmrow commented Nov 13, 2019

Fixed error text per @tjkirch !

@zmrow
Copy link
Contributor Author

zmrow commented Nov 13, 2019

Oh - and rebased as well. :)

Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

Approval stands!

@tjkirch tjkirch merged commit 7d01c07 into develop Nov 13, 2019
@tjkirch tjkirch deleted the meta-defaults branch November 13, 2019 22:54
@iliana iliana added this to the v0.2.0 milestone Nov 19, 2019
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.

Improve default settings representation
4 participants