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

[CCXDEV-14608] Add canary rollout for cluster-mapping.json file #324

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ FROM registry.access.redhat.com/ubi8/ubi-micro:latest
# copy the service
COPY --from=builder /opt/app-root/src/config.toml /config.toml
COPY --from=builder /opt/app-root/src/insights-operator-gathering-conditions-service .
COPY --from=builder /opt/app-root/src/cluster-mapping.json .
COPY --from=builder /opt/app-root/src/openapi.json .

# copy the certificates
Expand All @@ -56,6 +55,7 @@ COPY --from=builder /etc/pki /etc/pki
# copy the conditions
COPY --from=conditions /conditions /conditions
COPY --from=conditions /remote-configurations /remote-configurations
COPY --from=conditions /mapping /mapping

USER 1001

Expand Down
12 changes: 7 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ Service provides option of canary rollout for new version of configurations. The

## Configure

Configuration is done by `toml` config, taking the `config/config.toml` in the working directory if no other configuration is provided. This can be overriden by `INSIGHTS_OPERATOR_CONDITIONAL_SERVICE_CONFIG_FILE` environment variable.
Configuration is done by `toml` config, taking the `config.toml` in the working directory if no other configuration is provided. This can be overriden by `INSIGHTS_OPERATOR_CONDITIONAL_SERVICE_CONFIG_FILE` environment variable.

You can also override specific configuration options by setting specific
environment variables in this form:
Expand All @@ -96,17 +96,19 @@ For example, for a configuration like this

```
[storage]
remote_configuration = "./conditions/v2"
cluster_mapping = "cluster-mapping.json"
remote_configuration = "./conditions"
cluster_mapping = "./mapping"
```

you would override these values as follows:

```
export INSIGHTS_OPERATOR_GATHERING_CONDITIONS_SERVICE__STORAGE__REMOTE_CONFIGURATION=tests/rapid-recommendations
export INSIGHTS_OPERATOR_GATHERING_CONDITIONS_SERVICE__STORAGE__CLUSTER_MAPPING=tests/rapid-recommendations/cluster-mapping.json
export INSIGHTS_OPERATOR_GATHERING_CONDITIONS_SERVICE__STORAGE__CLUSTER_MAPPING=tests/mapping/
```

`remote_configuration`, `conditions` and `cluster_mapping` configuration options describe locations of respective content served by the service. However, each of these also contain separate `stable` and `canary` subdirectories containing different versions of the content. `cluster_mapping_file` option then describes the name of the file within `${cluster_mapping}/stable` and`${cluster_mapping}/canary`, and this file maps different OCP versions to the specific content under `${remote_configuration}/stable` (or `${remote_configuration}/canary`).

## Conditions

This service exposes the conditions from the
Expand Down Expand Up @@ -151,7 +153,7 @@ There are some flags for different purposes:
As part of [CCXDEV-12849](https://issues.redhat.com/browse/CCXDEV-12849) we
introduced a new feature to map remote configurations to different OCP versions.

In order to use it you need to set the `cluster_mapping` and
In order to use it you need to set the `cluster_mapping`, `cluster_mapping_file` and
`remote_configuration` fields in [config.toml] or use environment variables.
The cluster map should look like this:
```
Expand Down
3 changes: 0 additions & 3 deletions cluster-mapping.json

This file was deleted.

3 changes: 2 additions & 1 deletion config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ type = "jwt"
[storage]
rules_path = "./conditions"
remote_configuration = "./remote-configurations"
cluster_mapping = "cluster-mapping.json"
cluster_mapping = "./mapping"
cluster_mapping_file = "cluster-mapping.json"

[canary]
unleash_enabled = false
Expand Down
8 changes: 6 additions & 2 deletions get_conditions.sh
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
#!/bin/bash

STABLE_VERSION=1.1.0
CANARY_VERSION=1.1.0
STABLE_VERSION=1.1.2
CANARY_VERSION=1.1.2

# Clone the conditions repo and build it to gather the conditions
if [ ! -d 'insights-operator-gathering-conditions' ]; then git clone https://github.com/RedHatInsights/insights-operator-gathering-conditions; fi
mkdir -p conditions
mkdir -p remote-configurations
mkdir -p mapping/stable
mkdir -p mapping/canary
cd insights-operator-gathering-conditions || exit 1

git checkout ${STABLE_VERSION} && \
./build.sh && \
cp -r build/v1 ../conditions/stable && \
cp -r build/v2 ../remote-configurations/stable && \
cp build/cluster-mapping.json ../mapping/stable/ && \
rm -r build ; \

git checkout ${CANARY_VERSION} && \
./build.sh && \
cp -r build/v1 ../conditions/canary && \
cp -r build/v2 ../remote-configurations/canary && \
cp build/cluster-mapping.json ../mapping/canary/ && \
rm -r build ; \
32 changes: 21 additions & 11 deletions internal/service/cluster_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,29 @@ import (
)

// ClusterMapping map OCP versions to remote configurations
type ClusterMapping [][]string
type ClusterMapping struct {
version string
mapping [][]string
}

func NewClusterMapping(version string, mapping [][]string) *ClusterMapping {
return &ClusterMapping{
version: version,
mapping: mapping,
}
}

// IsValid check the list is in order (based on the versions), that the versions
// can be parsed and that the remote configurations are accessible
func (cm ClusterMapping) IsValid(remoteConfigurationPath string, conditionsVersion string) bool {
func (cm ClusterMapping) IsValid(remoteConfigurationPath string) bool {
versions := []semver.Version{} // used to check if it's sorted

if len(cm) == 0 {
log.Error().Interface("raw", cm).Msg("Cluster map needs to contain at least one pair of version and filepath")
if len(cm.mapping) == 0 {
log.Error().Interface("raw", cm.mapping).Msg("Cluster map needs to contain at least one pair of version and filepath")
return false
}

for _, slice := range cm {
for _, slice := range cm.mapping {
if len(slice) != 2 {
log.Error().Int("len", len(slice)).Strs("slice", slice).Msg("Unexpected slice length")
return false
Expand All @@ -38,7 +48,7 @@ func (cm ClusterMapping) IsValid(remoteConfigurationPath string, conditionsVersi
versions = append(versions, versionParsed)

filepath := slice[1]
fullFilepath := fmt.Sprintf("%s/%s/%s", remoteConfigurationPath, conditionsVersion, filepath)
fullFilepath := fmt.Sprintf("%s/%s/%s", remoteConfigurationPath, cm.version, filepath)
if _, err := os.Stat(fullFilepath); errors.Is(err, os.ErrNotExist) {
log.Error().Str("filepath", fullFilepath).
Msg("Remote configuration filepath couldn't be accessed")
Expand Down Expand Up @@ -72,7 +82,7 @@ func (cm ClusterMapping) IsValid(remoteConfigurationPath string, conditionsVersi
// than 3.0.0
func (cm ClusterMapping) GetFilepathForVersion(ocpVersionParsed semver.Version) (string, error) {
// check the version is not greater than the first slice
firstVersion, err := semver.Make(cm[0][0])
firstVersion, err := semver.Make(cm.mapping[0][0])
if err != nil {
log.Error().Str("version", firstVersion.String()).Err(err).Msg("Invalid semver")
return "", err
Expand All @@ -88,11 +98,11 @@ func (cm ClusterMapping) GetFilepathForVersion(ocpVersionParsed semver.Version)
return "", &merrors.NotFoundError{
ErrString: errMsg}
} else if comparison == 0 {
return cm[0][1], nil
return cm.mapping[0][1], nil
}

previousFilepath := cm[0][1]
for _, slice := range cm[1:] {
previousFilepath := cm.mapping[0][1]
for _, slice := range cm.mapping[1:] {
version := slice[0]
filepath := slice[1]
versionParsed, err := semver.Make(version)
Expand All @@ -115,5 +125,5 @@ func (cm ClusterMapping) GetFilepathForVersion(ocpVersionParsed semver.Version)

log.Debug().Str("ocpVersion", ocpVersionParsed.String()).
Msg("Returning latest remote configuration")
return cm[len(cm)-1][1], nil
return cm.mapping[len(cm.mapping)-1][1], nil
}
32 changes: 19 additions & 13 deletions internal/service/cluster_mapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,53 +13,58 @@ const testFilesPath = "../../tests/rapid-recommendations"

func TestClusterMappingIsValid(t *testing.T) {
t.Run("valid map", func(t *testing.T) {
var sut service.ClusterMapping = [][]string{
mapping := [][]string{
{"1.0.0", "experimental_1.json"},
{"2.0.0", "experimental_2.json"},
{"3.0.0", "config_default.json"},
}
assert.True(t, sut.IsValid(testFilesPath, service.StableVersion))
sut := service.NewClusterMapping(service.StableVersion, mapping)
assert.True(t, sut.IsValid(testFilesPath))
})

t.Run("invalid map: invalid version", func(t *testing.T) {
var sut service.ClusterMapping = [][]string{
mapping := [][]string{
{"1.0.0", "experimental_1.json"},
{"not a valid version", "experimental_2.json"},
{"3.0.0", "config_default.json"},
}
assert.False(t, sut.IsValid(testFilesPath, service.StableVersion))
sut := service.NewClusterMapping(service.StableVersion, mapping)
assert.False(t, sut.IsValid(testFilesPath))
})

t.Run("invalid map: JSON not found", func(t *testing.T) {
var sut service.ClusterMapping = [][]string{
mapping := [][]string{
{"1.0.0", "experimental_1.json"},
{"2.0.0", "not-found.json"},
{"3.0.0", "config_default.json"},
}
assert.False(t, sut.IsValid(testFilesPath, service.StableVersion))
sut := service.NewClusterMapping(service.StableVersion, mapping)
assert.False(t, sut.IsValid(testFilesPath))
})

t.Run("invalid map: versions unsorted", func(t *testing.T) {
var sut service.ClusterMapping = [][]string{
mapping := [][]string{
{"1.0.0", "experimental_1.json"},
{"3.0.0", "experimental_2.json"},
{"2.0.0", "config_default.json"},
}
assert.False(t, sut.IsValid(testFilesPath, service.StableVersion))
sut := service.NewClusterMapping(service.StableVersion, mapping)
assert.False(t, sut.IsValid(testFilesPath))
})

t.Run("invalid map: map contains a tripple instead of key-values pairs", func(t *testing.T) {
var sut service.ClusterMapping = [][]string{
mapping := [][]string{
{"1.0.0", "experimental_1.json", "some-element"},
{"2.0.0", "experimental_2.json"},
{"3.0.0", "config_default.json"},
}
assert.False(t, sut.IsValid(testFilesPath, service.StableVersion))
sut := service.NewClusterMapping(service.StableVersion, mapping)
assert.False(t, sut.IsValid(testFilesPath))
})

t.Run("invalid map: map is empty", func(t *testing.T) {
var sut service.ClusterMapping = [][]string{}
assert.False(t, sut.IsValid(testFilesPath, service.StableVersion))
sut := service.NewClusterMapping(service.StableVersion, [][]string{})
assert.False(t, sut.IsValid(testFilesPath))
})
}

Expand Down Expand Up @@ -102,11 +107,12 @@ func TestClusterMappingGetFilepathForVersion(t *testing.T) {
},
}

var sut service.ClusterMapping = [][]string{
mapping := [][]string{
{"1.0.0", "experimental_1.json"},
{"2.0.0", "experimental_2.json"},
{"3.0.0", "config_default.json"},
}
sut := service.NewClusterMapping(service.StableVersion, mapping)

for _, tc := range testCases {
t.Run(tc.ocpVersion, func(t *testing.T) {
Expand Down
10 changes: 7 additions & 3 deletions internal/service/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,18 @@ type mockStorage struct {
getRemoteConfigurationFilepathMockError error
}

func (m *mockStorage) ReadConditionalRules(*http.Request, string) []byte {
func (m *mockStorage) IsCanary(*http.Request) bool {
return true
}

func (m *mockStorage) ReadConditionalRules(bool, string) []byte {
return m.conditionalRules
}

func (m *mockStorage) ReadRemoteConfig(*http.Request, string) []byte {
func (m *mockStorage) ReadRemoteConfig(bool, string) []byte {
return m.remoteConfig
}

func (m *mockStorage) GetRemoteConfigurationFilepath(string) (string, error) {
func (m *mockStorage) GetRemoteConfigurationFilepath(bool, string) (string, error) {
return m.remoteConfigFilepath, m.getRemoteConfigurationFilepathMockError
}
7 changes: 4 additions & 3 deletions internal/service/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func NewRepository(s StorageInterface) *Repository {
// Rules method reads all and unmarshals all rules stored under given path
func (r *Repository) Rules(request *http.Request) (*Rules, error) {
filepath := "rules.json" // TODO: Make this configurable
data := r.store.ReadConditionalRules(request, filepath)
data := r.store.ReadConditionalRules(r.store.IsCanary(request), filepath)
if data == nil {
return nil, fmt.Errorf("store data not found for '%s'", filepath)
}
Expand All @@ -87,11 +87,12 @@ func (r *Repository) Rules(request *http.Request) (*Rules, error) {
// RemoteConfiguration returns a remote configuration for v2 endpoint based on
// the cluster map defined in the settings and loaded on startup
func (r *Repository) RemoteConfiguration(request *http.Request, ocpVersion string) (*RemoteConfiguration, error) {
filepath, err := r.store.GetRemoteConfigurationFilepath(ocpVersion)
isCanary := r.store.IsCanary(request)
filepath, err := r.store.GetRemoteConfigurationFilepath(isCanary, ocpVersion)
if err != nil {
return nil, err
}
data := r.store.ReadRemoteConfig(request, filepath)
data := r.store.ReadRemoteConfig(isCanary, filepath)
if data == nil {
return nil, fmt.Errorf("store data not found for '%s'", filepath)
}
Expand Down
Loading
Loading