Skip to content

Commit

Permalink
Merge pull request #5423 from QiWang19/storage-opts
Browse files Browse the repository at this point in the history
Bug 2012838: fix override storage options from storage.conf
  • Loading branch information
openshift-merge-robot committed Nov 4, 2021
2 parents d690832 + 8daa903 commit 3943334
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 0 deletions.
26 changes: 26 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,10 @@ func (c *Config) UpdateFromFile(path string) error {
// Returns errors encountered when reading or parsing the files, or nil
// otherwise.
func (c *Config) UpdateFromDropInFile(path string) error {
// keeps the storage options from storage.conf and merge it to crio config
var storageOpts []string
storageOpts = append(storageOpts, c.StorageOptions...)

data, err := ioutil.ReadFile(path)
if err != nil {
return err
Expand All @@ -592,6 +596,10 @@ func (c *Config) UpdateFromDropInFile(path string) error {
delete(c.Runtimes, defaultRuntime)
}

storageOpts = append(storageOpts, t.Crio.RootConfig.StorageOptions...)
storageOpts = removeDupStorageOpts(storageOpts)
t.Crio.RootConfig.StorageOptions = storageOpts

// Registries are deprecated in cri-o.conf and turned into a NOP.
// Users should use registries.conf instead, so let's log it.
if len(t.Crio.Image.Registries) > 0 {
Expand All @@ -603,6 +611,24 @@ func (c *Config) UpdateFromDropInFile(path string) error {
return nil
}

// removeDupStorageOpts removes duplicated storage option from the list
// keeps the last appearance
func removeDupStorageOpts(storageOpts []string) []string {
var resOpts []string
opts := make(map[string]bool)
for i := len(storageOpts) - 1; i >= 0; i-- {
if ok := opts[storageOpts[i]]; ok {
continue
}
opts[storageOpts[i]] = true
resOpts = append(resOpts, storageOpts[i])
}
for i, j := 0, len(resOpts)-1; i < j; i, j = i+1, j-1 {
resOpts[i], resOpts[j] = resOpts[j], resOpts[i]
}
return resOpts
}

// UpdateFromPath recursively iterates the provided path and updates the
// configuration for it
func (c *Config) UpdateFromPath(path string) error {
Expand Down
31 changes: 31 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,37 @@ var _ = t.Describe("Config", func() {
Expect(sut.PidsLimit).To(BeEquivalentTo(2048))
})

It("should inherit storage_options from storage.conf and remove duplicates", func() {
f := t.MustTempFile("config")
// Given
Expect(ioutil.WriteFile(f,
[]byte(`
[crio]
storage_option = [
"foo=bar",
]`,
), 0),
).To(BeNil())
for _, tc := range []struct {
opts []string
expect []string
}{
{[]string{"option1=v1", "option2=v2", "option3=v3"}, []string{"option1=v1", "option2=v2", "option3=v3", "foo=bar"}},
{[]string{"option1=v1", "option3=v3", "option2=v2", "option3=v3"}, []string{"option1=v1", "option2=v2", "option3=v3", "foo=bar"}},
{[]string{"option1=v1", "option2=v2", "option3=v3", "option1=v1"}, []string{"option2=v2", "option3=v3", "option1=v1", "foo=bar"}},
{[]string{"option1=v1", "option2=v2", "option3=v3", "option4=v4", "option3=v3", "option1=v1"}, []string{"option2=v2", "option4=v4", "option3=v3", "option1=v1", "foo=bar"}},
} {
// When
defaultcfg := defaultConfig()
defaultcfg.StorageOptions = tc.opts
err := defaultcfg.UpdateFromFile(f)

// Then
Expect(err).To(BeNil())
Expect(defaultcfg.RootConfig.StorageOptions).To(Equal(tc.expect))
}
})

It("should succeed with custom runtime", func() {
// Given
f := t.MustTempFile("config")
Expand Down

0 comments on commit 3943334

Please sign in to comment.