Merge pull request #324 from JiriPapousek/cluster-mapping-canary-rollout

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

Dockerfile

@@ -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
@@ -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
 

README.md

@@ -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:
@@ -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
@@ -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:
 ```

config.toml

@@ -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

get_conditions.sh

@@ -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 ; \

internal/service/cluster_mapping.go

@@ -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
@@ -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")
@@ -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
@@ -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)
@@ -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
 }

internal/service/cluster_mapping_test.go

@@ -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))
 	})
 }
 
@@ -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) {

internal/service/common_test.go

@@ -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
 }

internal/service/repository.go

@@ -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)
 	}
@@ -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)
 	}