From 43247e3e5fd4a4ae8740d13c170b009402206b03 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Thu, 14 Jan 2021 08:20:17 -0500 Subject: [PATCH] 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()