PAUSED: Use git resume to continue working.

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()

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 {

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 {

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)

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()
+}

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

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)