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

add openshift ex config patch to modify master-config.yaml #9165

Merged
merged 1 commit into from
Jun 7, 2016

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Jun 3, 2016

Introduces openshift ex config patch path/to/master-config.yaml --patch '{}' to allow master config mutation without using sed. The scheme changes and patch flag additions may take us a little while to work out, this gives us a removable command to play with and see if it suits us.

@fabianofranz
@soltysh @stevekuznetsov

@deads2k
Copy link
Contributor Author

deads2k commented Jun 3, 2016

[test]

@soltysh
Copy link
Contributor

soltysh commented Jun 3, 2016

LGTM

@deads2k
Copy link
Contributor Author

deads2k commented Jun 3, 2016

[merge]

@fabianofranz
Copy link
Member

+1 LGTM

@deads2k
Copy link
Contributor Author

deads2k commented Jun 4, 2016

yum re[merge]

@smarterclayton
Copy link
Contributor

When would I use this over improvements to the other patch?

@deads2k
Copy link
Contributor Author

deads2k commented Jun 6, 2016

When would I use this over improvements to the other patch?

Two problems:

  1. Our config objects exist in a different scheme and a lot of code, include patch, rely on a single scheme. It's not insurmountable, but not something I want to rush into either. I'd want to create a new API group and find a way to decode using a "trust me" serialized version.
  2. The upstream patch changes looks like they're going to take a little while. I think they'll come around, but this is blocking our ability to fully test conformance on different configs now (audit in particular).

I figured on having an experimental command until we get everything sorted out in a few weeks and pick up changes in another rebase.

@stevekuznetsov
Copy link
Contributor

stevekuznetsov commented Jun 6, 2016

but this is blocking our ability to fully test conformance

To be fair, this is the Golang program that we can write to alleviate our block:

package main

import (
    "io/ioutil"
    "os"

    "gopkg.in/yaml.v2"

    "github.com/openshift/origin/pkg/cmd/server/api/v1"
)

func main() {
    var config v1.MasterConfig

    rawInput, err := ioutil.ReadFile(os.Args[1])
    if err != nil {
        panic(err)
    }

    err = yaml.Unmarshal(rawInput, &config)
    if err != nil {
        panic(err)
    }

    config.AuditConfig.Enabled = true

    rawOutput, err := yaml.Marshal(config)
    if err != nil {
        panic(err)
    }

    err = ioutil.WriteFile(os.Args[1]+".updated", rawOutput, os.O_WRONLY|O_CREATE)
    if err != nil {
        panic(err)
    }
}

@mfojtik
Copy link
Contributor

mfojtik commented Jun 6, 2016

yey! 👍

@smarterclayton
Copy link
Contributor

smarterclayton commented Jun 6, 2016 via email

@deads2k
Copy link
Contributor Author

deads2k commented Jun 6, 2016

That's fine - I would expect though that the upstream patch command
replace this command.

Agreed. yum re[merge]

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2016
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2016
@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 5920c0b

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4496/)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 5920c0b

@openshift-bot
Copy link
Contributor

openshift-bot commented Jun 7, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4496/) (Image: devenv-rhel7_4324)

@openshift-bot openshift-bot merged commit e5553b9 into openshift:master Jun 7, 2016
@rhcarvalho rhcarvalho mentioned this pull request Jun 28, 2016
@deads2k deads2k deleted the config-patch branch September 6, 2016 17:18
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.

7 participants