From d621041485b4b32aa81b42a9c31338f6742046ed Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Tue, 12 Jan 2021 13:53:11 -0500 Subject: [PATCH 1/6] Move tempfile handling into create() --- internal/isoeditor/rhcos.go | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/internal/isoeditor/rhcos.go b/internal/isoeditor/rhcos.go index 5f8c334ed8f..738d479884d 100644 --- a/internal/isoeditor/rhcos.go +++ b/internal/isoeditor/rhcos.go @@ -77,29 +77,25 @@ func (e *rhcosEditor) CreateMinimalISOTemplate(serviceBaseURL string) (string, e return "", err } + e.log.Info("Creating minimal ISO template") + return e.create() +} + +func (e *rhcosEditor) create() (string, error) { isoPath, err := tempFileName() if err != nil { return "", err } - e.log.Infof("Creating minimal ISO template: %s", isoPath) - if err := e.create(isoPath); err != nil { - return "", err - } - - return isoPath, nil -} - -func (e *rhcosEditor) create(outPath string) error { volumeID, err := e.isoHandler.VolumeIdentifier() if err != nil { - return err + return "", err } - if err = e.isoHandler.Create(outPath, volumeID); err != nil { - return err + if err = e.isoHandler.Create(isoPath, volumeID); err != nil { + return "", err } - return nil + return isoPath, nil } func (e *rhcosEditor) addRootFSURL(serviceBaseURL string) error { From 077c21507935df86edfa69d64aeea0db6a6ef940 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 23 Dec 2020 17:00:06 -0500 Subject: [PATCH 2/6] WIP MGMT-2977 Generate cluster-specific minimal iso This will take a cluster and add the files specified in the discovery ignition patch to the iso as an additional initrd image. This will allow these customizations to take effect before the rootfs is downloaded --- internal/isoeditor/rhcos.go | 117 ++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/internal/isoeditor/rhcos.go b/internal/isoeditor/rhcos.go index 738d479884d..50140369aca 100644 --- a/internal/isoeditor/rhcos.go +++ b/internal/isoeditor/rhcos.go @@ -4,11 +4,18 @@ import ( "fmt" "io/ioutil" "os" + "path/filepath" "regexp" "github.com/openshift/assisted-service/internal/isoutil" "github.com/openshift/assisted-service/restapi/operations/bootfiles" + + "github.com/cavaliercoder/go-cpio" + config_31 "github.com/coreos/ignition/v2/config/v3_1" + config_31_types "github.com/coreos/ignition/v2/config/v3_1/types" "github.com/sirupsen/logrus" + "github.com/thoas/go-funk" + "github.com/vincent-petithory/dataurl" ) const ( @@ -18,6 +25,7 @@ const ( type Editor interface { CreateMinimalISOTemplate(serviceBaseURL string) (string, error) + CreateClusterMinimalISO(ignition string) (string, error) } type rhcosEditor struct { @@ -81,6 +89,115 @@ func (e *rhcosEditor) CreateMinimalISOTemplate(serviceBaseURL string) (string, e return e.create() } +// CreateClusterMinimalISO creates a new rhcos iso with cluser file customizations added +// to the initrd image +func (e *rhcosEditor) CreateClusterMinimalISO(ignition string) (string, error) { + if err := e.isoHandler.Extract(); err != nil { + return "", err + } + defer func() { + if err := e.isoHandler.CleanWorkDir(); err != nil { + e.log.WithError(err).Warnf("Failed to clean isoHandler work dir") + } + }() + + if err := e.addIgnitionFiles(ignition); err != nil { + return "", err + } + + return e.create() +} + +func addFile(w *cpio.Writer, f config_31_types.File) error { + u, err := dataurl.DecodeString(f.Contents.Key()) + if err != nil { + return err + } + + var mode cpio.FileMode = 0644 + if f.Mode != nil { + mode = cpio.FileMode(*f.Mode) + } + + uid := 0 + if f.User.ID != nil { + uid = *f.User.ID + } + + gid := 0 + if f.Group.ID != nil { + gid = *f.Group.ID + } + + // add the file + hdr := &cpio.Header{ + Name: f.Path, + Mode: mode, + UID: uid, + GID: gid, + Size: int64(len(u.Data)), + } + if err := w.WriteHeader(hdr); err != nil { + return err + } + if _, err := w.Write(u.Data); err != nil { + return err + } + + return nil +} + +// addIgnitionFiles adds all files referenced in the given ignition config to +// the initrd by creating an additional cpio archive +func (e *rhcosEditor) addIgnitionFiles(ignition string) error { + config, _, err := config_31.Parse([]byte(ignition)) + if err != nil { + return err + } + + f, err := os.Create(e.isoHandler.ExtractedPath("images/assisted_custom_files.img")) + if err != nil { + return fmt.Errorf("failed to open image file: %s", err) + } + + w := cpio.NewWriter(f) + addedPaths := make([]string, 0) + + // TODO: deal with config.Storage.Directories also? + for _, f := range config.Storage.Files { + if err = addFile(w, f); err != nil { + return fmt.Errorf("failed to add file %s to archive: %v", f.Path, err) + } + + // Need to add all directories in the file path to ensure it can be created + // Many files may be in the same directory so we need to track which directories we add to ensure we only add them once + for dir := filepath.Dir(f.Path); dir != "" && dir != "/"; dir = filepath.Dir(dir) { + if !funk.Contains(addedPaths, dir) { + hdr := &cpio.Header{ + Name: dir, + Mode: 040755, + Size: 0, + } + if err = w.WriteHeader(hdr); err != nil { + return err + } + addedPaths = append(addedPaths, dir) + } + } + } + + if err = w.Close(); err != nil { + return err + } + + // edit configs to add new image + err = editFile(e.isoHandler.ExtractedPath("EFI/redhat/grub.cfg"), `(?m)^(\s+initrd) (.+| )+$`, "$1 $2 /images/assisted_custom_files.img") + if err != nil { + return err + } + return editFile(e.isoHandler.ExtractedPath("isolinux/isolinux.cfg"), `(?m)^(\s+append.*initrd=\S+) (.*)$`, "${1},/images/assisted_custom_files.img ${2}") +} + func (e *rhcosEditor) create() (string, error) { isoPath, err := tempFileName() if err != nil { From 7b64be533776968f7f36e8d2864819effdbf3794 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Tue, 12 Jan 2021 14:37:35 -0500 Subject: [PATCH 3/6] MGMT-2977 Add discovery image type This adds a type enum to both the cluster image info and the image creation parameters --- models/image_create_params.go | 23 +++++++++++++ models/image_info.go | 23 +++++++++++++ models/image_type.go | 63 +++++++++++++++++++++++++++++++++++ restapi/embedded_spec.go | 32 ++++++++++++++++++ swagger.yaml | 11 ++++++ 5 files changed, 152 insertions(+) create mode 100644 models/image_type.go diff --git a/models/image_create_params.go b/models/image_create_params.go index 7d9f2a93dd0..64bc1ebd066 100644 --- a/models/image_create_params.go +++ b/models/image_create_params.go @@ -18,6 +18,9 @@ import ( // swagger:model image-create-params type ImageCreateParams struct { + // Type of image that should be generated. + ImageType ImageType `json:"image_type,omitempty"` + // SSH public key for debugging the installation. SSHPublicKey string `json:"ssh_public_key,omitempty"` @@ -29,6 +32,10 @@ type ImageCreateParams struct { func (m *ImageCreateParams) Validate(formats strfmt.Registry) error { var res []error + if err := m.validateImageType(formats); err != nil { + res = append(res, err) + } + if err := m.validateStaticIpsConfig(formats); err != nil { res = append(res, err) } @@ -39,6 +46,22 @@ func (m *ImageCreateParams) Validate(formats strfmt.Registry) error { return nil } +func (m *ImageCreateParams) validateImageType(formats strfmt.Registry) error { + + if swag.IsZero(m.ImageType) { // not required + return nil + } + + if err := m.ImageType.Validate(formats); err != nil { + if ve, ok := err.(*errors.Validation); ok { + return ve.ValidateName("image_type") + } + return err + } + + return nil +} + func (m *ImageCreateParams) validateStaticIpsConfig(formats strfmt.Registry) error { if swag.IsZero(m.StaticIpsConfig) { // not required diff --git a/models/image_info.go b/models/image_info.go index bdd56606d59..9ee6ff8e65b 100644 --- a/models/image_info.go +++ b/models/image_info.go @@ -40,6 +40,9 @@ type ImageInfo struct { // statip ips configuration string in the format expected by discovery ignition StaticIpsConfig string `json:"static_ips_config,omitempty"` + + // type + Type ImageType `json:"type,omitempty"` } // Validate validates this image info @@ -58,6 +61,10 @@ func (m *ImageInfo) Validate(formats strfmt.Registry) error { res = append(res, err) } + if err := m.validateType(formats); err != nil { + res = append(res, err) + } + if len(res) > 0 { return errors.CompositeValidationError(res...) } @@ -103,6 +110,22 @@ func (m *ImageInfo) validateSizeBytes(formats strfmt.Registry) error { return nil } +func (m *ImageInfo) validateType(formats strfmt.Registry) error { + + if swag.IsZero(m.Type) { // not required + return nil + } + + if err := m.Type.Validate(formats); err != nil { + if ve, ok := err.(*errors.Validation); ok { + return ve.ValidateName("type") + } + return err + } + + return nil +} + // MarshalBinary interface implementation func (m *ImageInfo) MarshalBinary() ([]byte, error) { if m == nil { diff --git a/models/image_type.go b/models/image_type.go new file mode 100644 index 00000000000..75a3b65467b --- /dev/null +++ b/models/image_type.go @@ -0,0 +1,63 @@ +// Code generated by go-swagger; DO NOT EDIT. + +package models + +// This file was generated by the swagger tool. +// Editing this file might prove futile when you re-run the swagger generate command + +import ( + "encoding/json" + + "github.com/go-openapi/errors" + "github.com/go-openapi/strfmt" + "github.com/go-openapi/validate" +) + +// ImageType image type +// +// swagger:model image_type +type ImageType string + +const ( + + // ImageTypeFullIso captures enum value "full-iso" + ImageTypeFullIso ImageType = "full-iso" + + // ImageTypeMinimalIso captures enum value "minimal-iso" + ImageTypeMinimalIso ImageType = "minimal-iso" +) + +// for schema +var imageTypeEnum []interface{} + +func init() { + var res []ImageType + if err := json.Unmarshal([]byte(`["full-iso","minimal-iso"]`), &res); err != nil { + panic(err) + } + for _, v := range res { + imageTypeEnum = append(imageTypeEnum, v) + } +} + +func (m ImageType) validateImageTypeEnum(path, location string, value ImageType) error { + if err := validate.EnumCase(path, location, value, imageTypeEnum, true); err != nil { + return err + } + return nil +} + +// Validate validates this image type +func (m ImageType) Validate(formats strfmt.Registry) error { + var res []error + + // value enum + if err := m.validateImageTypeEnum("", "body", m); err != nil { + return err + } + + if len(res) > 0 { + return errors.CompositeValidationError(res...) + } + return nil +} diff --git a/restapi/embedded_spec.go b/restapi/embedded_spec.go index cbc3d6109ac..bf207ea838a 100644 --- a/restapi/embedded_spec.go +++ b/restapi/embedded_spec.go @@ -5711,6 +5711,11 @@ func init() { "image-create-params": { "type": "object", "properties": { + "image_type": { + "description": "Type of image that should be generated.", + "type": "string", + "$ref": "#/definitions/image_type" + }, "ssh_public_key": { "description": "SSH public key for debugging the installation.", "type": "string" @@ -5753,9 +5758,20 @@ func init() { "static_ips_config": { "description": "statip ips configuration string in the format expected by discovery ignition", "type": "string" + }, + "type": { + "type": "string", + "$ref": "#/definitions/image_type" } } }, + "image_type": { + "type": "string", + "enum": [ + "full-iso", + "minimal-iso" + ] + }, "infra_error": { "type": "object", "required": [ @@ -12080,6 +12096,11 @@ func init() { "image-create-params": { "type": "object", "properties": { + "image_type": { + "description": "Type of image that should be generated.", + "type": "string", + "$ref": "#/definitions/image_type" + }, "ssh_public_key": { "description": "SSH public key for debugging the installation.", "type": "string" @@ -12123,9 +12144,20 @@ func init() { "static_ips_config": { "description": "statip ips configuration string in the format expected by discovery ignition", "type": "string" + }, + "type": { + "type": "string", + "$ref": "#/definitions/image_type" } } }, + "image_type": { + "type": "string", + "enum": [ + "full-iso", + "minimal-iso" + ] + }, "infra_error": { "type": "object", "required": [ diff --git a/swagger.yaml b/swagger.yaml index b83fb325c72..ce286988447 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -2921,6 +2921,10 @@ definitions: type: array items: $ref: '#/definitions/static-ip-config' + image_type: + type: string + description: Type of image that should be generated. + $ref: '#/definitions/image_type' assisted-service-iso-create-params: type: object @@ -3695,6 +3699,13 @@ definitions: static_ips_config: type: string description: statip ips configuration string in the format expected by discovery ignition + type: + type: string + $ref: '#/definitions/image_type' + + image_type: + type: string + enum: [full-iso, minimal-iso] free-addresses-list: type: array From fe12cd4a4f5a5b0b9a71c6dc3e3f1a1589f55dd2 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Tue, 12 Jan 2021 17:05:20 -0500 Subject: [PATCH 4/6] WIP Use the minimal template iso when minimal-iso image type is requested --- internal/bminventory/inventory.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/internal/bminventory/inventory.go b/internal/bminventory/inventory.go index 3e88c2c818e..152907185c5 100644 --- a/internal/bminventory/inventory.go +++ b/internal/bminventory/inventory.go @@ -833,7 +833,7 @@ func (b *bareMetalInventory) DownloadClusterISO(ctx context.Context, params inst contentLength) } -func (b *bareMetalInventory) updateImageInfoPostUpload(ctx context.Context, cluster *common.Cluster, clusterProxyHash string) error { +func (b *bareMetalInventory) updateImageInfoPostUpload(ctx context.Context, cluster *common.Cluster, clusterProxyHash string, imageType models.ImageType) error { updates := map[string]interface{}{} imgName := getImageName(*cluster.ID) imgSize, err := b.objectHandler.GetObjectSizeBytes(ctx, imgName) @@ -858,6 +858,9 @@ func (b *bareMetalInventory) updateImageInfoPostUpload(ctx context.Context, clus cluster.ProxyHash = clusterProxyHash } + updates["image_type"] = imageType + cluster.ImageInfo.Type = imageType + dbReply := b.db.Model(&common.Cluster{}).Where("id = ?", cluster.ID.String()).Updates(updates) if dbReply.Error != nil { return errors.New("Failed to generate image: error updating image record") @@ -981,7 +984,7 @@ func (b *bareMetalInventory) GenerateClusterISOInternal(ctx context.Context, par } if imageExists { - if err := b.updateImageInfoPostUpload(ctx, &cluster, clusterProxyHash); err != nil { + if err := b.updateImageInfoPostUpload(ctx, &cluster, clusterProxyHash, params.ImageCreateParams.ImageType); err != nil { return nil, common.NewApiError(http.StatusInternalServerError, err) } @@ -1002,14 +1005,19 @@ func (b *bareMetalInventory) GenerateClusterISOInternal(ctx context.Context, par return nil, common.NewApiError(http.StatusInternalServerError, err) } - if err := b.objectHandler.UploadISO(ctx, ignitionConfig, b.objectHandler.GetBaseIsoObject(cluster.OpenshiftVersion), - fmt.Sprintf(s3wrapper.DiscoveryImageTemplate, cluster.ID.String())); err != nil { + baseISOName := b.objectHandler.GetBaseIsoObject(cluster.OpenshiftVersion) + if params.ImageCreateParams.ImageType == models.ImageTypeMinimalIso { + baseISOName = s3wrapper.GetMinimalISOObjectName(cluster.OpenshiftVersion) + } + objectPrefix := fmt.Sprintf(s3wrapper.DiscoveryImageTemplate, cluster.ID.String()) + + if err := b.objectHandler.UploadISO(ctx, ignitionConfig, baseISOName, objectPrefix); err != nil { log.WithError(err).Errorf("Upload ISO failed for cluster %s", cluster.ID) b.eventsHandler.AddEvent(ctx, params.ClusterID, nil, models.EventSeverityError, "Failed to upload image", time.Now()) return nil, common.NewApiError(http.StatusInternalServerError, err) } - if err := b.updateImageInfoPostUpload(ctx, &cluster, clusterProxyHash); err != nil { + if err := b.updateImageInfoPostUpload(ctx, &cluster, clusterProxyHash, params.ImageCreateParams.ImageType); err != nil { return nil, common.NewApiError(http.StatusInternalServerError, err) } From 60a435ee40890061c1a5dc247bbe404518851024 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 13 Jan 2021 13:20:31 -0500 Subject: [PATCH 5/6] Move logic for creating ignition archive to isoeditor This will make it easier to share between the existing logic which creates the archive then uploads it to s3 and the new logic which will write the archive to the fs before bundling it into the minimal iso directly --- internal/isoeditor/rhcos.go | 33 ++++++++++++++++++++++++++++++ pkg/s3wrapper/upload_iso.go | 40 ++++++------------------------------- 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/internal/isoeditor/rhcos.go b/internal/isoeditor/rhcos.go index 50140369aca..72b9a3860e8 100644 --- a/internal/isoeditor/rhcos.go +++ b/internal/isoeditor/rhcos.go @@ -1,6 +1,8 @@ package isoeditor import ( + "bytes" + "compress/gzip" "fmt" "io/ioutil" "os" @@ -13,6 +15,7 @@ import ( "github.com/cavaliercoder/go-cpio" config_31 "github.com/coreos/ignition/v2/config/v3_1" config_31_types "github.com/coreos/ignition/v2/config/v3_1/types" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/thoas/go-funk" "github.com/vincent-petithory/dataurl" @@ -256,3 +259,33 @@ func tempFileName() (string, error) { return path, nil } + +func IgnitionImageArchive(ignitionConfig string) ([]byte, error) { + ignitionBytes := []byte(ignitionConfig) + + // Create CPIO archive + archiveBuffer := new(bytes.Buffer) + cpioWriter := cpio.NewWriter(archiveBuffer) + if err := cpioWriter.WriteHeader(&cpio.Header{Name: "config.ign", Mode: 0o100_644, Size: int64(len(ignitionBytes))}); err != nil { + return nil, errors.Wrap(err, "Failed to write CPIO header") + } + if _, err := cpioWriter.Write(ignitionBytes); err != nil { + + return nil, errors.Wrap(err, "Failed to write CPIO archive") + } + if err := cpioWriter.Close(); err != nil { + return nil, errors.Wrap(err, "Failed to close CPIO archive") + } + + // Run gzip compression + compressedBuffer := new(bytes.Buffer) + gzipWriter := gzip.NewWriter(compressedBuffer) + if _, err := gzipWriter.Write(archiveBuffer.Bytes()); err != nil { + return nil, errors.Wrap(err, "Failed to gzip ignition config") + } + if err := gzipWriter.Close(); err != nil { + return nil, errors.Wrap(err, "Failed to gzip ignition config") + } + + return compressedBuffer.Bytes(), nil +} diff --git a/pkg/s3wrapper/upload_iso.go b/pkg/s3wrapper/upload_iso.go index 25c35302a03..788b7b6390b 100644 --- a/pkg/s3wrapper/upload_iso.go +++ b/pkg/s3wrapper/upload_iso.go @@ -2,7 +2,6 @@ package s3wrapper import ( "bytes" - "compress/gzip" "context" "encoding/binary" "fmt" @@ -11,11 +10,10 @@ import ( "sort" "sync" - "github.com/cavaliercoder/go-cpio" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" "github.com/aws/aws-sdk-go/service/s3/s3iface" + "github.com/openshift/assisted-service/internal/isoeditor" logutil "github.com/openshift/assisted-service/pkg/log" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -384,45 +382,19 @@ func (m *multiUpload) uploadPartCopy(c chunk) error { } func (m *multiUpload) uploadIgnition(log logrus.FieldLogger, partNum int64, ignitionConfig string) error { - ignitionBytes := []byte(ignitionConfig) - - // Create CPIO archive - archiveBuffer := new(bytes.Buffer) - cpioWriter := cpio.NewWriter(archiveBuffer) - if err := cpioWriter.WriteHeader(&cpio.Header{Name: "config.ign", Mode: 0o100_644, Size: int64(len(ignitionBytes))}); err != nil { - m.log.WithError(err).Errorf("Failed to write CPIO header") - return err - } - if _, err := cpioWriter.Write(ignitionBytes); err != nil { - m.log.WithError(err).Errorf("Failed to write CPIO archive") - return err - } - if err := cpioWriter.Close(); err != nil { - m.log.WithError(err).Errorf("Failed to close CPIO archive") - return err - } - - // Run gzip compression - compressedBuffer := new(bytes.Buffer) - gzipWriter := gzip.NewWriter(compressedBuffer) - if _, err := gzipWriter.Write(archiveBuffer.Bytes()); err != nil { - err = errors.Wrapf(err, "Failed to gzip ignition config") - m.log.Error(err) - return err - } - if err := gzipWriter.Close(); err != nil { - err = errors.Wrapf(err, "Failed to gzip ignition config") + imageBytes, err := isoeditor.IgnitionImageArchive(ignitionConfig) + if err != nil { m.log.Error(err) return err } - if int64(len(compressedBuffer.Bytes())) > m.isoInfo.areaLengthBytes { - err := errors.New(fmt.Sprintf("Ignition is too long to be embedded (%d > %d)", len(compressedBuffer.Bytes()), m.isoInfo.areaLengthBytes)) + if int64(len(imageBytes)) > m.isoInfo.areaLengthBytes { + err = errors.New(fmt.Sprintf("Ignition is too long to be embedded (%d > %d)", len(imageBytes), m.isoInfo.areaLengthBytes)) m.log.Error(err) return err } - copy(*m.origContents, compressedBuffer.Bytes()) + copy(*m.origContents, imageBytes) contentLength := int64(len(*m.origContents)) completedPartCopy, err := m.uploader.s3client.UploadPart(&s3.UploadPartInput{ From 43247e3e5fd4a4ae8740d13c170b009402206b03 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Thu, 14 Jan 2021 08:20:17 -0500 Subject: [PATCH 6/6] PAUSED: Use `git resume` to continue working. --- cmd/main.go | 3 ++ internal/bminventory/inventory.go | 44 +++++++++++++++++++++------ internal/isoeditor/rhcos.go | 23 +++++++++++++++ pkg/s3wrapper/client.go | 5 ++++ pkg/s3wrapper/file_cache.go | 49 ++++++++++++++++++++++++------- pkg/s3wrapper/file_cache_test.go | 30 ++++++++++++++++--- pkg/s3wrapper/filesystem.go | 4 +++ pkg/s3wrapper/mock_s3wrapper.go | 16 ++++++++++ 8 files changed, 151 insertions(+), 23 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index f1159ba13c6..b9d3b66ea77 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -7,6 +7,7 @@ import ( "fmt" "log" "net/http" + "os" "strings" "time" @@ -154,6 +155,8 @@ func main() { log.Println(fmt.Sprintf("Started service with OCP versions %v", openshiftVersionsMap)) + failOnError(os.MkdirAll(Options.BMConfig.ISOCacheDir, 0700), "Failed to create ISO cache directory %s", Options.BMConfig.ISOCacheDir) + // Connect to db db := setupDB(log) defer db.Close() diff --git a/internal/bminventory/inventory.go b/internal/bminventory/inventory.go index 152907185c5..a0defe90bf1 100644 --- a/internal/bminventory/inventory.go +++ b/internal/bminventory/inventory.go @@ -18,6 +18,7 @@ import ( "net" "net/http" "net/url" + "os" "regexp" "sort" "strconv" @@ -42,6 +43,7 @@ import ( "github.com/openshift/assisted-service/internal/identity" "github.com/openshift/assisted-service/internal/ignition" "github.com/openshift/assisted-service/internal/installcfg" + "github.com/openshift/assisted-service/internal/isoeditor" "github.com/openshift/assisted-service/internal/manifests" "github.com/openshift/assisted-service/internal/metrics" "github.com/openshift/assisted-service/internal/network" @@ -96,6 +98,7 @@ type Config struct { ServiceIPs string `envconfig:"SERVICE_IPS" default:""` DeletedUnregisteredAfter time.Duration `envconfig:"DELETED_UNREGISTERED_AFTER" default:"168h"` DefaultNTPSource string `envconfig:"NTP_DEFAULT_SERVER"` + ISOCacheDir string `envconfig:"ISO_CACHE_DIR" default:"/tmp/isocache"` } const agentMessageOfTheDay = ` @@ -943,7 +946,8 @@ func (b *bareMetalInventory) GenerateClusterISOInternal(ctx context.Context, par var imageExists bool if cluster.ImageInfo.SSHPublicKey == params.ImageCreateParams.SSHPublicKey && cluster.ProxyHash == clusterProxyHash && - cluster.ImageInfo.StaticIpsConfig == staticIpsConfig { + cluster.ImageInfo.StaticIpsConfig == staticIpsConfig && + cluster.ImageInfo.Type == params.ImageCreateParams.ImageType { var err error imgName := getImageName(params.ClusterID) imageExists, err = b.objectHandler.UpdateObjectTimestamp(ctx, imgName) @@ -1005,16 +1009,38 @@ func (b *bareMetalInventory) GenerateClusterISOInternal(ctx context.Context, par return nil, common.NewApiError(http.StatusInternalServerError, err) } - baseISOName := b.objectHandler.GetBaseIsoObject(cluster.OpenshiftVersion) - if params.ImageCreateParams.ImageType == models.ImageTypeMinimalIso { - baseISOName = s3wrapper.GetMinimalISOObjectName(cluster.OpenshiftVersion) - } objectPrefix := fmt.Sprintf(s3wrapper.DiscoveryImageTemplate, cluster.ID.String()) - if err := b.objectHandler.UploadISO(ctx, ignitionConfig, baseISOName, objectPrefix); err != nil { - log.WithError(err).Errorf("Upload ISO failed for cluster %s", cluster.ID) - b.eventsHandler.AddEvent(ctx, params.ClusterID, nil, models.EventSeverityError, "Failed to upload image", time.Now()) - return nil, common.NewApiError(http.StatusInternalServerError, err) + if params.ImageCreateParams.ImageType == models.ImageTypeMinimalIso { + baseISOName := s3wrapper.GetMinimalIsoObjectName(cluster.OpenshiftVersion) + isoPath, err := s3wrapper.GetFile(ctx, b.objectHandler, baseISOName, b.ISOCacheDir, true) + if err != nil { + log.WithError(err).Errorf("Failed to download minimal ISO template %s", baseISOName) + return nil, common.NewApiError(http.StatusInternalServerError, err) + } + + log.Infof("Creating minimal ISO for cluster %s", cluster.ID) + clusterISOPath, err := isoeditor.CreateEditor(isoPath, cluster.OpenshiftVersion, log).CreateClusterMinimalISO(ignitionConfig) + if err != nil { + log.WithError(err).Errorf("Failed to create minimal discovery ISO for cluster %s", cluster.ID) + return nil, common.NewApiError(http.StatusInternalServerError, err) + } + + log.Infof("Uploading minimal ISO for cluster %s", cluster.ID) + if err := b.objectHandler.UploadFile(ctx, clusterISOPath, fmt.Sprintf("%s.iso", objectPrefix)); err != nil { + os.Remove(clusterISOPath) + log.WithError(err).Errorf("Failed to upload minimal discovery ISO for cluster %s", cluster.ID) + return nil, common.NewApiError(http.StatusInternalServerError, err) + } + os.Remove(clusterISOPath) + } else { + baseISOName := b.objectHandler.GetBaseIsoObject(cluster.OpenshiftVersion) + + if err := b.objectHandler.UploadISO(ctx, ignitionConfig, baseISOName, objectPrefix); err != nil { + log.WithError(err).Errorf("Upload ISO failed for cluster %s", cluster.ID) + b.eventsHandler.AddEvent(ctx, params.ClusterID, nil, models.EventSeverityError, "Failed to upload image", time.Now()) + return nil, common.NewApiError(http.StatusInternalServerError, err) + } } if err := b.updateImageInfoPostUpload(ctx, &cluster, clusterProxyHash, params.ImageCreateParams.ImageType); err != nil { diff --git a/internal/isoeditor/rhcos.go b/internal/isoeditor/rhcos.go index 72b9a3860e8..7b47edac455 100644 --- a/internal/isoeditor/rhcos.go +++ b/internal/isoeditor/rhcos.go @@ -108,9 +108,22 @@ func (e *rhcosEditor) CreateClusterMinimalISO(ignition string) (string, error) { return "", err } + if err := e.addIgnitionArchive(ignition); err != nil { + return "", err + } + return e.create() } +func (e *rhcosEditor) addIgnitionArchive(ignition string) error { + archiveBytes, err := IgnitionImageArchive(ignition) + if err != nil { + return err + } + + return ioutil.WriteFile(e.isoHandler.ExtractedPath("images/ignition.img"), archiveBytes, 0644) +} + func addFile(w *cpio.Writer, f config_31_types.File) error { u, err := dataurl.DecodeString(f.Contents.Key()) if err != nil { @@ -193,6 +206,16 @@ func (e *rhcosEditor) addIgnitionFiles(ignition string) error { return err } + err = editFile(e.isoHandler.ExtractedPath("EFI/redhat/grub.cfg"), ` coreos.liveiso=\S+`, "") + if err != nil { + return err + } + + err = editFile(e.isoHandler.ExtractedPath("isolinux/isolinux.cfg"), ` coreos.liveiso=\S+`, "") + if err != nil { + return err + } + // edit configs to add new image err = editFile(e.isoHandler.ExtractedPath("EFI/redhat/grub.cfg"), `(?m)^(\s+initrd) (.+| )+$`, "$1 $2 /images/assisted_custom_files.img") if err != nil { diff --git a/pkg/s3wrapper/client.go b/pkg/s3wrapper/client.go index df2507bd09e..dadd28b28c2 100644 --- a/pkg/s3wrapper/client.go +++ b/pkg/s3wrapper/client.go @@ -65,6 +65,7 @@ type API interface { UploadStreamToPublicBucket(ctx context.Context, reader io.Reader, objectName string) error UploadFileToPublicBucket(ctx context.Context, filePath, objectName string) error DoesPublicObjectExist(ctx context.Context, objectName string) (bool, error) + DownloadPublic(ctx context.Context, objectName string) (io.ReadCloser, int64, error) } var _ API = &S3Client{} @@ -281,6 +282,10 @@ func (c *S3Client) Download(ctx context.Context, objectName string) (io.ReadClos return c.download(ctx, objectName, c.cfg.S3Bucket, c.client) } +func (c *S3Client) DownloadPublic(ctx context.Context, objectName string) (io.ReadCloser, int64, error) { + return c.download(ctx, objectName, c.cfg.PublicS3Bucket, c.client) +} + func (c *S3Client) doesObjectExist(ctx context.Context, objectName, bucket string, client s3iface.S3API) (bool, error) { log := logutil.FromContext(ctx, c.log) log.Debugf("Verifying if %s exists in %s", objectName, bucket) diff --git a/pkg/s3wrapper/file_cache.go b/pkg/s3wrapper/file_cache.go index 5e9eefda4de..c680aa2aee3 100644 --- a/pkg/s3wrapper/file_cache.go +++ b/pkg/s3wrapper/file_cache.go @@ -9,7 +9,6 @@ import ( "sync" ) -// list of files keyed by the s3 object id type fileList struct { sync.Mutex files map[string]*file @@ -24,25 +23,42 @@ var cache fileList = fileList{ files: make(map[string]*file), } -func (l *fileList) get(objectName string) *file { +func (l *fileList) get(key string) *file { l.Lock() defer l.Unlock() - f, present := l.files[objectName] + f, present := l.files[key] if !present { f = &file{} - l.files[objectName] = f + l.files[key] = f } return f } -func downloadFile(ctx context.Context, objectHandler API, objectName string, cacheDir string) (string, error) { - reader, size, err := objectHandler.Download(ctx, objectName) +func (l *fileList) clear() { + l.Lock() + defer l.Unlock() + + l.files = make(map[string]*file) +} + +func downloadFile(ctx context.Context, objectHandler API, objectName string, cacheDir string, public bool) (string, error) { + var ( + reader io.ReadCloser + err error + size int64 + ) + + if public { + reader, size, err = objectHandler.DownloadPublic(ctx, objectName) + } else { + reader, size, err = objectHandler.Download(ctx, objectName) + } if err != nil { return "", err } - f, err := os.Create(filepath.Join(cacheDir, objectName)) + f, err := os.Create(filepath.Join(cacheDir, cacheKey(objectName, public))) if err != nil { return "", err } @@ -58,14 +74,23 @@ func downloadFile(ctx context.Context, objectHandler API, objectName string, cac return f.Name(), nil } -func GetFile(ctx context.Context, objectHandler API, objectName string, cacheDir string) (string, error) { - f := cache.get(objectName) +func cacheKey(objectName string, public bool) string { + prefix := "private" + if public { + prefix = "public" + } + + return fmt.Sprintf("%s-%s", prefix, objectName) +} + +func GetFile(ctx context.Context, objectHandler API, objectName string, cacheDir string, public bool) (string, error) { + f := cache.get(cacheKey(objectName, public)) f.Lock() defer f.Unlock() //cache miss if f.path == "" { - path, err := downloadFile(ctx, objectHandler, objectName, cacheDir) + path, err := downloadFile(ctx, objectHandler, objectName, cacheDir, public) if err != nil { return "", err } @@ -74,3 +99,7 @@ func GetFile(ctx context.Context, objectHandler API, objectName string, cacheDir return f.path, nil } + +func ClearFileCache() { + cache.clear() +} diff --git a/pkg/s3wrapper/file_cache_test.go b/pkg/s3wrapper/file_cache_test.go index c7c1faa63b6..5df8b0956b8 100644 --- a/pkg/s3wrapper/file_cache_test.go +++ b/pkg/s3wrapper/file_cache_test.go @@ -30,6 +30,7 @@ var _ = Describe("GetFile", func() { AfterEach(func() { ctrl.Finish() os.RemoveAll(cacheDir) + ClearFileCache() }) It("Downloads files only when not present in the cache", func() { @@ -45,23 +46,44 @@ var _ = Describe("GetFile", func() { r2 := ioutil.NopCloser(strings.NewReader(content2)) mockAPI.EXPECT().Download(ctx, objName2).Times(1).Return(r2, int64(len(content2)), nil) - path1, err := GetFile(ctx, mockAPI, objName1, cacheDir) + path1, err := GetFile(ctx, mockAPI, objName1, cacheDir, false) Expect(err).ToNot(HaveOccurred()) validateFileContent(path1, content1) - path2, err := GetFile(ctx, mockAPI, objName2, cacheDir) + path2, err := GetFile(ctx, mockAPI, objName2, cacheDir, false) Expect(err).ToNot(HaveOccurred()) validateFileContent(path2, content2) // get both files again to ensure download isn't called more than once - path1, err = GetFile(ctx, mockAPI, objName1, cacheDir) + path1, err = GetFile(ctx, mockAPI, objName1, cacheDir, false) Expect(err).ToNot(HaveOccurred()) validateFileContent(path1, content1) - path2, err = GetFile(ctx, mockAPI, objName2, cacheDir) + path2, err = GetFile(ctx, mockAPI, objName2, cacheDir, false) Expect(err).ToNot(HaveOccurred()) validateFileContent(path2, content2) }) + + It("Keeps separate cache entries for public vs private", func() { + ctx := context.Background() + + objName := "my-test-object" + content := "hello world" + r := ioutil.NopCloser(strings.NewReader(content)) + mockAPI.EXPECT().Download(ctx, objName).Times(1).Return(r, int64(len(content)), nil) + + contentPub := "HELLO WORLD" + rPub := ioutil.NopCloser(strings.NewReader(contentPub)) + mockAPI.EXPECT().DownloadPublic(ctx, objName).Times(1).Return(rPub, int64(len(contentPub)), nil) + + path, err := GetFile(ctx, mockAPI, objName, cacheDir, false) + Expect(err).ToNot(HaveOccurred()) + validateFileContent(path, content) + + pathPub, err := GetFile(ctx, mockAPI, objName, cacheDir, true) + Expect(err).ToNot(HaveOccurred()) + validateFileContent(pathPub, contentPub) + }) }) func validateFileContent(path string, content string) { diff --git a/pkg/s3wrapper/filesystem.go b/pkg/s3wrapper/filesystem.go index 6e476ef2c1d..75b38468d4f 100644 --- a/pkg/s3wrapper/filesystem.go +++ b/pkg/s3wrapper/filesystem.go @@ -177,6 +177,10 @@ func (f *FSClient) Download(ctx context.Context, objectName string) (io.ReadClos return ioutils.NewReadCloserWrapper(fp, fp.Close), info.Size(), nil } +func (f *FSClient) DownloadPublic(ctx context.Context, objectName string) (io.ReadCloser, int64, error) { + return f.Download(ctx, objectName) +} + func (f *FSClient) DoesObjectExist(ctx context.Context, objectName string) (bool, error) { filePath := filepath.Join(f.basedir, objectName) info, err := os.Stat(filePath) diff --git a/pkg/s3wrapper/mock_s3wrapper.go b/pkg/s3wrapper/mock_s3wrapper.go index 3cbce10221e..0db944cc3b9 100644 --- a/pkg/s3wrapper/mock_s3wrapper.go +++ b/pkg/s3wrapper/mock_s3wrapper.go @@ -157,6 +157,22 @@ func (mr *MockAPIMockRecorder) DownloadBootFile(arg0, arg1, arg2 interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DownloadBootFile", reflect.TypeOf((*MockAPI)(nil).DownloadBootFile), arg0, arg1, arg2) } +// DownloadPublic mocks base method +func (m *MockAPI) DownloadPublic(arg0 context.Context, arg1 string) (io.ReadCloser, int64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DownloadPublic", arg0, arg1) + ret0, _ := ret[0].(io.ReadCloser) + ret1, _ := ret[1].(int64) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// DownloadPublic indicates an expected call of DownloadPublic +func (mr *MockAPIMockRecorder) DownloadPublic(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DownloadPublic", reflect.TypeOf((*MockAPI)(nil).DownloadPublic), arg0, arg1) +} + // ExpireObjects mocks base method func (m *MockAPI) ExpireObjects(arg0 context.Context, arg1 string, arg2 time.Duration, arg3 func(context.Context, logrus.FieldLogger, string)) { m.ctrl.T.Helper()