Skip to content

Commit

Permalink
MGMT-14704: Provide info on custom/vs non custom manifest in manifest…
Browse files Browse the repository at this point in the history
… endpoint.

It has been reported that users have no way to determine whether or not a manifest is a custom manifest or one generated by the assisted-service.
This PR introduces a change that uses the file system to store additional metadata about a manifest to indicate whether or not the manifest is a custom one.

For example, if the original file is stored in

```
"e09df13f-6f31-42c2-8361-2b5605f80e77/manifests/openshift/99-openshift-machineconfig-master-kargs.yaml"
```

Then, if an only if the manifest is custom -  a corresponding metadata file will be created in the following path.

```
"e09df13f-6f31-42c2-8361-2b5605f80e77/manifest-attributes/openshift/99-openshift-machineconfig-master-kargs.yaml/user-defined"
```

Any internally generated manifests are considered to be "non custom" and are created as such.
All other manifests are considered to be custom and will follow the scheme above.

When the user retrieves the manifest list, an additional parameter will be supplied for each manifest to indicate the custom/non custom status of the manifest.
  • Loading branch information
paul-maidment committed Jun 8, 2023
1 parent d2a504a commit b979dea
Show file tree
Hide file tree
Showing 20 changed files with 239 additions and 61 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion internal/cluster/cluster.go
Expand Up @@ -1415,7 +1415,7 @@ func (m *Manager) deleteClusterFiles(ctx context.Context, c *common.Cluster, obj
var failedToDelete []string
for _, file := range files {
//skip log and manifests deletion when deleting cluster files
if folder == "" && (strings.Contains(file, "logs") || strings.Contains(file, "manifests")) {
if folder == "" && (strings.Contains(file, "logs") || strings.Contains(file, "manifests") || strings.Contains(file, "manifest-attributes")) {
continue
}
log.Debugf("Deleting cluster %s S3 file: %s", c.ID.String(), file)
Expand All @@ -1438,6 +1438,10 @@ func (m *Manager) deleteClusterManifests(ctx context.Context, c *common.Cluster,
return m.deleteClusterFiles(ctx, c, objectHandler, "manifests")
}

func (m *Manager) deleteClusterManifestAttributes(ctx context.Context, c *common.Cluster, objectHandler s3wrapper.API) error {
return m.deleteClusterFiles(ctx, c, objectHandler, "manifest-attributes")
}

func (m *Manager) DeleteClusterLogs(ctx context.Context, c *common.Cluster, objectHandler s3wrapper.API) error {
return m.deleteClusterFiles(ctx, c, objectHandler, "logs")
}
Expand Down Expand Up @@ -1487,6 +1491,10 @@ func (m Manager) PermanentClustersDeletion(ctx context.Context, olderThan strfmt
deleteFromDB = false
m.log.WithError(err).Warnf("Failed deleting s3 manifests of cluster %s", c.ID.String())
}
if err := m.deleteClusterManifestAttributes(ctx, c, objectHandler); err != nil {
deleteFromDB = false
m.log.WithError(err).Warnf("Failed deleting s3 manifest-attributes of cluster %s", c.ID.String())
}
if _, err := objectHandler.DeleteObject(ctx, c.ID.String()); err != nil {
deleteFromDB = false
m.log.WithError(err).Warnf("Failed deleting cluster directory %s", c.ID.String())
Expand Down
Expand Up @@ -1156,7 +1156,8 @@ func (r *ClusterDeploymentsReconciler) syncManifests(ctx context.Context, log lo
Content: swag.String(base64.StdEncoding.EncodeToString([]byte(manifest))),
FileName: swag.String(filename),
Folder: swag.String(models.ManifestFolderOpenshift),
}})
}},
true)
if err != nil {
log.WithError(err).Errorf("Failed to create cluster deployment %s manifest %s", cluster.KubeKeyName, filename)
return err
Expand Down
Expand Up @@ -1975,7 +1975,7 @@ var _ = Describe("cluster reconcile", func() {
mockInstallerInternal.EXPECT().HostWithCollectedLogsExists(gomock.Any()).Return(false, nil)
mockInstallerInternal.EXPECT().GetKnownHostApprovedCounts(gomock.Any()).Return(5, 5, nil).Times(2)
mockManifestsApi.EXPECT().ListClusterManifestsInternal(gomock.Any(), gomock.Any()).Return(models.ListManifests{}, nil).Times(1)
mockManifestsApi.EXPECT().CreateClusterManifestInternal(gomock.Any(), gomock.Any()).Return(nil, errors.Errorf("error")).Times(1)
mockManifestsApi.EXPECT().CreateClusterManifestInternal(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.Errorf("error")).Times(1)
request := newClusterDeploymentRequest(cluster)
aci = getTestClusterInstall()
aci.Spec.ManifestsConfigMapRef = ref
Expand Down Expand Up @@ -2039,7 +2039,7 @@ var _ = Describe("cluster reconcile", func() {

mockInstallerInternal.EXPECT().GetClusterByKubeKey(gomock.Any()).Return(backEndCluster, nil)
mockInstallerInternal.EXPECT().ValidatePullSecret(gomock.Any(), gomock.Any()).Return(nil).Times(2)
mockManifestsApi.EXPECT().CreateClusterManifestInternal(gomock.Any(), gomock.Any()).Return(nil, nil).Times(1)
mockManifestsApi.EXPECT().CreateClusterManifestInternal(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).Times(1)
mockClusterApi.EXPECT().IsReadyForInstallation(gomock.Any()).Return(true, "").Times(1)
mockManifestsApi.EXPECT().ListClusterManifestsInternal(gomock.Any(), gomock.Any()).Return(models.ListManifests{}, nil).Times(1)
mockInstallerInternal.EXPECT().GetKnownHostApprovedCounts(gomock.Any()).Return(5, 5, nil).Times(1)
Expand Down Expand Up @@ -2164,7 +2164,7 @@ var _ = Describe("cluster reconcile", func() {

mockInstallerInternal.EXPECT().GetClusterByKubeKey(gomock.Any()).Return(backEndCluster, nil)
mockInstallerInternal.EXPECT().ValidatePullSecret(gomock.Any(), gomock.Any()).Return(nil).Times(2)
mockManifestsApi.EXPECT().CreateClusterManifestInternal(gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
mockManifestsApi.EXPECT().CreateClusterManifestInternal(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).Times(2)
mockClusterApi.EXPECT().IsReadyForInstallation(gomock.Any()).Return(true, "").Times(1)
mockManifestsApi.EXPECT().ListClusterManifestsInternal(gomock.Any(), gomock.Any()).Return(models.ListManifests{}, nil).Times(1)
mockInstallerInternal.EXPECT().GetKnownHostApprovedCounts(gomock.Any()).Return(5, 5, nil).Times(1)
Expand Down
2 changes: 1 addition & 1 deletion internal/manifests/api/interface.go
Expand Up @@ -16,7 +16,7 @@ type ManifestsAPI interface {

//go:generate mockgen --build_flags=--mod=mod -package api -destination mock_manifests_internal.go . ClusterManifestsInternals
type ClusterManifestsInternals interface {
CreateClusterManifestInternal(ctx context.Context, params operations.V2CreateClusterManifestParams) (*models.Manifest, error)
CreateClusterManifestInternal(ctx context.Context, params operations.V2CreateClusterManifestParams, isCustomManifest bool) (*models.Manifest, error)
ListClusterManifestsInternal(ctx context.Context, params operations.V2ListClusterManifestsParams) (models.ListManifests, error)
DeleteClusterManifestInternal(ctx context.Context, params operations.V2DeleteClusterManifestParams) error
}
8 changes: 4 additions & 4 deletions internal/manifests/api/mock_manifests_api.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions internal/manifests/api/mock_manifests_internal.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

109 changes: 104 additions & 5 deletions internal/manifests/manifests.go
Expand Up @@ -34,6 +34,7 @@ var _ manifestsapi.ManifestsAPI = &Manifests{}

// ManifestFolder represents the manifests folder on s3 per cluster
const ManifestFolder = "manifests"
const ManifestMetadataFolder = "manifest-attributes"

// NewManifestsAPI returns manifests API
func NewManifestsAPI(db *gorm.DB, log logrus.FieldLogger, objectHandler s3wrapper.API, usageAPI usage.API) *Manifests {
Expand All @@ -53,7 +54,12 @@ type Manifests struct {
usageAPI usage.API
}

func (m *Manifests) CreateClusterManifestInternal(ctx context.Context, params operations.V2CreateClusterManifestParams) (*models.Manifest, error) {
const (
ManifestSourceCustomManifest string = "custom-manifest"
ManifestSourceGenerated string = "generated"
)

func (m *Manifests) CreateClusterManifestInternal(ctx context.Context, params operations.V2CreateClusterManifestParams, isCustomManifest bool) (*models.Manifest, error) {
log := logutil.FromContext(ctx, m.log)
log.Infof("Creating manifest in cluster %s", params.ClusterID.String())

Expand Down Expand Up @@ -90,11 +96,74 @@ func (m *Manifests) CreateClusterManifestInternal(ctx context.Context, params op
return nil, err
}

err = m.createMetadataForManifest(ctx, params.ClusterID, path, isCustomManifest)
if err != nil {
return nil, err
}

log.Infof("Done creating manifest %s for cluster %s", path, params.ClusterID.String())
manifest := models.Manifest{FileName: fileName, Folder: folder}
manifest := models.Manifest{FileName: fileName, Folder: folder, IsCustomManifest: &isCustomManifest}
return &manifest, nil
}

func (m *Manifests) isCustomManifest(ctx context.Context, clusterID string, folder string, fileName string) *bool {
var is_custom_manifest *bool
manifestMetadataPrefix := filepath.Join(clusterID, ManifestMetadataFolder)
for _, manifestSourceKey := range []string{ManifestSourceCustomManifest, ManifestSourceGenerated} {
manifestSourceValue, err := m.objectHandler.DoesObjectExist(ctx, filepath.Join(manifestMetadataPrefix, folder, fileName, manifestSourceKey))
if err != nil {
m.log.Warnf("Could not determine if manifest %s/%s is a %s manifest due to error %s, leaving this as nil", folder, fileName, manifestSourceKey, err.Error())
}
switch manifestSourceKey {
case ManifestSourceCustomManifest:
if manifestSourceValue {
trueValue := true
is_custom_manifest = &trueValue
}
case ManifestSourceGenerated:
if manifestSourceValue {
falseValue := false
is_custom_manifest = &falseValue
}
default:
is_custom_manifest = nil
}
}
return is_custom_manifest
}

func (m *Manifests) deleteMetadataForManifest(ctx context.Context, clusterID strfmt.UUID, path string) error {
for _, manifestSource := range []string{ManifestSourceCustomManifest, ManifestSourceGenerated} {
objectName := GetManifestMetadataObjectName(clusterID, path, manifestSource)
exists := false
var err error
if exists, err = m.objectHandler.DoesObjectExist(ctx, objectName); err != nil {
return errors.Wrapf(err, "Failed to delete metadata object %s from storage for cluster %s", objectName, clusterID)
}
if exists {
if _, err = m.objectHandler.DeleteObject(ctx, objectName); err != nil {
return errors.Wrapf(err, "Failed to delete metadata object %s from storage for cluster %s", objectName, clusterID)
}
}
}
return nil
}

func (m *Manifests) createMetadataForManifest(ctx context.Context, clusterID strfmt.UUID, path string, isCustomManifest bool) error {
log := logutil.FromContext(ctx, m.log)
log.Infof("Is custom manifest %t", isCustomManifest)
manifestSource := ManifestSourceGenerated
if isCustomManifest {
manifestSource = ManifestSourceCustomManifest
}
log.Infof("Adding manifest source %s metadata to cluster %s", manifestSource, clusterID)
objectName := GetManifestMetadataObjectName(clusterID, path, manifestSource)
if err := m.objectHandler.Upload(ctx, []byte{}, objectName); err != nil {
return err
}
return nil
}

func (m *Manifests) ListClusterManifestsInternal(ctx context.Context, params operations.V2ListClusterManifestsParams) (models.ListManifests, error) {
log := logutil.FromContext(ctx, m.log)
log.Debugf("Listing manifests in cluster %s", params.ClusterID)
Expand All @@ -118,12 +187,14 @@ func (m *Manifests) ListClusterManifestsInternal(ctx context.Context, params ope
for _, file := range files {
parts := strings.Split(strings.Trim(file, string(filepath.Separator)), string(filepath.Separator))
if len(parts) > 2 {
manifests = append(manifests, &models.Manifest{FileName: filepath.Join(parts[3:]...), Folder: parts[2]})
fileName := filepath.Join(parts[3:]...)
folder := parts[2]
is_custom_manifest := m.isCustomManifest(ctx, params.ClusterID.String(), folder, fileName)
manifests = append(manifests, &models.Manifest{FileName: fileName, Folder: folder, IsCustomManifest: is_custom_manifest})
} else {
return nil, common.NewApiError(http.StatusInternalServerError, errors.Errorf("Cannot list file %s in cluster %s", file, params.ClusterID.String()))
}
}

return manifests, nil
}

Expand All @@ -149,11 +220,17 @@ func (m *Manifests) DeleteClusterManifestInternal(ctx context.Context, params op
return err
}

err = m.deleteMetadataForManifest(ctx, params.ClusterID, path)
if err != nil {
return err
}

log.Infof("Done deleting cluster manifest %s for cluster %s", path, params.ClusterID.String())
return nil
}

func (m *Manifests) UpdateClusterManifestInternal(ctx context.Context, params operations.V2UpdateClusterManifestParams) (*models.Manifest, error) {
m.log.Infof("Call to update cluster manifest internal")
if params.UpdateManifestParams.UpdatedFolder == nil {
params.UpdateManifestParams.UpdatedFolder = &params.UpdateManifestParams.Folder
}
Expand All @@ -163,6 +240,9 @@ func (m *Manifests) UpdateClusterManifestInternal(ctx context.Context, params op
srcFolder, srcFileName, srcPath := m.getManifestPathsFromParameters(ctx, &params.UpdateManifestParams.Folder, &params.UpdateManifestParams.FileName)
destFolder, destFileName, destPath := m.getManifestPathsFromParameters(ctx, params.UpdateManifestParams.UpdatedFolder, params.UpdateManifestParams.UpdatedFileName)

m.log.Infof("srcFolder: %s, srcFileName: %s, srcPath: %s", srcFolder, srcFileName, srcPath)
m.log.Infof("destFolder: %s, destFileName: %s, destPath: %s", destFolder, destFileName, destPath)

cluster, err := common.GetClusterFromDB(m.db, params.ClusterID, common.SkipEagerLoading)
if err != nil {
err = fmt.Errorf("Object Not Found")
Expand Down Expand Up @@ -202,16 +282,30 @@ func (m *Manifests) UpdateClusterManifestInternal(ctx context.Context, params op
return nil, err
}

isCustomManifest := m.isCustomManifest(ctx, params.ClusterID.String(), srcFolder, srcFileName)
if isCustomManifest != nil {
err = m.createMetadataForManifest(ctx, params.ClusterID, destPath, *isCustomManifest)
if err != nil {
return nil, err
}
}

if srcPath != destPath {
m.log.Infof("Removing old manifest %s for cluster %s as the new file is at %s", srcPath, params.ClusterID.String(), destPath)
err = m.deleteManifest(ctx, params.ClusterID, srcPath)
if err != nil {
return nil, err
}
if isCustomManifest != nil {
err = m.deleteMetadataForManifest(ctx, params.ClusterID, srcPath)
if err != nil {
return nil, err
}
}
}

m.log.Infof("Done creating manifest %s for cluster %s", destFileName, params.ClusterID.String())
manifest := models.Manifest{FileName: destFileName, Folder: destFolder}
manifest := models.Manifest{FileName: destFileName, Folder: destFolder, IsCustomManifest: isCustomManifest}
return &manifest, nil
}

Expand Down Expand Up @@ -279,6 +373,11 @@ func GetManifestObjectName(clusterID strfmt.UUID, fileName string) string {
return filepath.Join(string(clusterID), ManifestFolder, fileName)
}

// GetManifestObjectName returns the manifest object name as stored in S3
func GetManifestMetadataObjectName(clusterID strfmt.UUID, fileName string, manifestSource string) string {
return filepath.Join(string(clusterID), ManifestMetadataFolder, fileName, manifestSource)
}

// GetClusterManifests returns a list of cluster manifests
func GetClusterManifests(ctx context.Context, clusterID *strfmt.UUID, s3Client s3wrapper.API) ([]string, error) {
manifestFiles := []string{}
Expand Down

0 comments on commit b979dea

Please sign in to comment.