From dd375cdf0b45ce9af9ec2c85305f211ddb1c95a3 Mon Sep 17 00:00:00 2001 From: staebler Date: Tue, 11 Dec 2018 16:23:37 -0500 Subject: [PATCH 1/7] asset/manifest: do not load Common Manifest dependent manifests from disk The network, ingress, and dns manifest assets are not targeted assets. The installer does not write their files to disk. The installer should not load their files from disk. In addition to the load issue, there is an issue with reading the config field when reading the assets from the state file. For the network asset, the config field is needed, but is not stored in the state file. To resolve this, the config field has been made public. For the ingress and dns assets, the config field is not needed: it has been removed. Fixes https://jira.coreos.com/browse/CORS-938 --- pkg/asset/manifests/dns.go | 34 +++------------------------ pkg/asset/manifests/ingress.go | 36 ++++------------------------ pkg/asset/manifests/network.go | 43 +++++++--------------------------- 3 files changed, 15 insertions(+), 98 deletions(-) diff --git a/pkg/asset/manifests/dns.go b/pkg/asset/manifests/dns.go index ccfce56766b..4373b36e976 100644 --- a/pkg/asset/manifests/dns.go +++ b/pkg/asset/manifests/dns.go @@ -1,7 +1,6 @@ package manifests import ( - "os" "path/filepath" "github.com/ghodss/yaml" @@ -22,7 +21,6 @@ var ( // DNS generates the cluster-dns-*.yml files. type DNS struct { - config *configv1.DNS FileList []*asset.File } @@ -46,7 +44,7 @@ func (d *DNS) Generate(dependencies asset.Parents) error { installConfig := &installconfig.InstallConfig{} dependencies.Get(installConfig) - d.config = &configv1.DNS{ + config := &configv1.DNS{ TypeMeta: metav1.TypeMeta{ APIVersion: configv1.SchemeGroupVersion.String(), Kind: "DNS", @@ -60,7 +58,7 @@ func (d *DNS) Generate(dependencies asset.Parents) error { }, } - configData, err := yaml.Marshal(d.config) + configData, err := yaml.Marshal(config) if err != nil { return errors.Wrapf(err, "failed to create %s manifests from InstallConfig", d.Name()) } @@ -91,31 +89,5 @@ func (d *DNS) Files() []*asset.File { // Load loads the already-rendered files back from disk. func (d *DNS) Load(f asset.FileFetcher) (bool, error) { - crdFile, err := f.FetchByName(filepath.Join(manifestDir, dnsCrdFilename)) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - - cfgFile, err := f.FetchByName(dnsCfgFilename) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - - return false, err - } - - dnsConfig := &configv1.DNS{} - if err := yaml.Unmarshal(cfgFile.Data, dnsConfig); err != nil { - return false, errors.Wrapf(err, "failed to unmarshal %s", dnsCfgFilename) - } - - fileList := []*asset.File{crdFile, cfgFile} - - d.FileList, d.config = fileList, dnsConfig - - return true, nil + return false, nil } diff --git a/pkg/asset/manifests/ingress.go b/pkg/asset/manifests/ingress.go index 0a17fba6682..8478f5b46ff 100644 --- a/pkg/asset/manifests/ingress.go +++ b/pkg/asset/manifests/ingress.go @@ -2,7 +2,6 @@ package manifests import ( "fmt" - "os" "path/filepath" "github.com/ghodss/yaml" @@ -23,7 +22,6 @@ var ( // Ingress generates the cluster-ingress-*.yml files. type Ingress struct { - config *configv1.Ingress FileList []*asset.File } @@ -47,7 +45,7 @@ func (ing *Ingress) Generate(dependencies asset.Parents) error { installConfig := &installconfig.InstallConfig{} dependencies.Get(installConfig) - ing.config = &configv1.Ingress{ + config := &configv1.Ingress{ TypeMeta: metav1.TypeMeta{ APIVersion: configv1.SchemeGroupVersion.String(), Kind: "Ingress", @@ -61,7 +59,7 @@ func (ing *Ingress) Generate(dependencies asset.Parents) error { }, } - configData, err := yaml.Marshal(ing.config) + configData, err := yaml.Marshal(config) if err != nil { return errors.Wrapf(err, "failed to create %s manifests from InstallConfig", ing.Name()) } @@ -90,33 +88,7 @@ func (ing *Ingress) Files() []*asset.File { return ing.FileList } -// Load loads the already-rendered files back from disk. +// Load returns false since this asset is not written to disk by the installer. func (ing *Ingress) Load(f asset.FileFetcher) (bool, error) { - crdFile, err := f.FetchByName(filepath.Join(manifestDir, ingCrdFilename)) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - - cfgFile, err := f.FetchByName(ingCfgFilename) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - - return false, err - } - - ingressConfig := &configv1.Ingress{} - if err := yaml.Unmarshal(cfgFile.Data, ingressConfig); err != nil { - return false, errors.Wrapf(err, "failed to unmarshal %s", ingCfgFilename) - } - - fileList := []*asset.File{crdFile, cfgFile} - - ing.FileList, ing.config = fileList, ingressConfig - - return true, nil + return false, nil } diff --git a/pkg/asset/manifests/network.go b/pkg/asset/manifests/network.go index 8fd338c887b..55801da20fb 100644 --- a/pkg/asset/manifests/network.go +++ b/pkg/asset/manifests/network.go @@ -2,7 +2,6 @@ package manifests import ( "fmt" - "os" "path/filepath" "github.com/ghodss/yaml" @@ -33,7 +32,7 @@ var ( // Networking generates the cluster-network-*.yml files. type Networking struct { - config *configv1.Network + Config *configv1.Network FileList []*asset.File } @@ -74,7 +73,7 @@ func (no *Networking) Generate(dependencies asset.Parents) error { return errors.Errorf("ClusterNetworks must be specified") } - no.config = &configv1.Network{ + no.Config = &configv1.Network{ TypeMeta: metav1.TypeMeta{ APIVersion: configv1.SchemeGroupVersion.String(), Kind: "Network", @@ -90,7 +89,7 @@ func (no *Networking) Generate(dependencies asset.Parents) error { }, } - configData, err := yaml.Marshal(no.config) + configData, err := yaml.Marshal(no.Config) if err != nil { return errors.Wrapf(err, "failed to create %s manifests from InstallConfig", no.Name()) } @@ -123,19 +122,19 @@ func (no *Networking) Files() []*asset.File { // object. This is called by ClusterK8sIO, which captures generalized cluster // state but shouldn't need to be fully networking aware. func (no *Networking) ClusterNetwork() (*clusterv1a1.ClusterNetworkingConfig, error) { - if no.config == nil { + if no.Config == nil { // should be unreachable. return nil, errors.Errorf("ClusterNetwork called before initialization") } pods := []string{} - for _, cn := range no.config.Spec.ClusterNetwork { + for _, cn := range no.Config.Spec.ClusterNetwork { pods = append(pods, cn.CIDR) } cn := &clusterv1a1.ClusterNetworkingConfig{ Services: clusterv1a1.NetworkRanges{ - CIDRBlocks: no.config.Spec.ServiceNetwork, + CIDRBlocks: no.Config.Spec.ServiceNetwork, }, Pods: clusterv1a1.NetworkRanges{ CIDRBlocks: pods, @@ -144,33 +143,7 @@ func (no *Networking) ClusterNetwork() (*clusterv1a1.ClusterNetworkingConfig, er return cn, nil } -// Load loads the already-rendered files back from disk. +// Load returns false since this asset is not written to disk by the installer. func (no *Networking) Load(f asset.FileFetcher) (bool, error) { - crdFile, err := f.FetchByName(noCrdFilename) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, err - } - - cfgFile, err := f.FetchByName(noCfgFilename) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - - return false, err - } - - netConfig := &configv1.Network{} - if err := yaml.Unmarshal(cfgFile.Data, netConfig); err != nil { - return false, errors.Wrapf(err, "failed to unmarshal %s", noCfgFilename) - } - - fileList := []*asset.File{crdFile, cfgFile} - - no.FileList, no.config = fileList, netConfig - - return true, nil + return false, nil } From b25db3d4010af9620298d640871c60d27ba27201 Mon Sep 17 00:00:00 2001 From: staebler Date: Wed, 12 Dec 2018 16:02:14 -0500 Subject: [PATCH 2/7] asset/templates: remove fileName field The fileName field is not needed. The template assets do not need to store this information. The problem with the fileName field is that it is not set when the asset is loaded from the state file or from on-disk. This causes round-trip tests that compare a generated asset to a read asset to fail. Changes for https://jira.coreos.com/browse/CORS-940 --- .../bootkube/04-openshift-machine-config-operator.go | 7 +++---- .../bootkube/05-openshift-cluster-api-namespace.go | 7 +++---- .../09-openshift-service-cert-signer-namespace.go | 7 +++---- pkg/asset/templates/content/bootkube/cvo-overrides.go | 7 +++---- pkg/asset/templates/content/bootkube/etcd-service.go | 7 +++---- .../content/bootkube/host-etcd-service-endpoints.go | 7 +++---- .../templates/content/bootkube/host-etcd-service.go | 7 +++---- .../templates/content/bootkube/kube-cloud-config.go | 7 +++---- .../bootkube/kube-system-configmap-etcd-serving-ca.go | 7 +++---- .../content/bootkube/kube-system-configmap-root-ca.go | 7 +++---- .../content/bootkube/kube-system-secret-etcd-client.go | 7 +++---- .../bootkube/machine-config-server-tls-secret.go | 7 +++---- .../openshift-service-cert-signer-ca-secret.go | 7 +++---- pkg/asset/templates/content/bootkube/pull.go | 7 +++---- .../templates/content/openshift/binding-discovery.go | 7 +++---- .../templates/content/openshift/cloud-creds-secret.go | 7 +++---- .../content/openshift/cluster-infrastructure-crd.go | 10 ++++------ .../content/openshift/kubeadmin-password-secret.go | 7 +++---- .../openshift/role-cloud-creds-secret-reader.go | 7 +++---- 19 files changed, 58 insertions(+), 78 deletions(-) diff --git a/pkg/asset/templates/content/bootkube/04-openshift-machine-config-operator.go b/pkg/asset/templates/content/bootkube/04-openshift-machine-config-operator.go index 3b5676ddc52..a304ec5b835 100644 --- a/pkg/asset/templates/content/bootkube/04-openshift-machine-config-operator.go +++ b/pkg/asset/templates/content/bootkube/04-openshift-machine-config-operator.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*OpenshiftMachineConfigOperator)(nil) // OpenshiftMachineConfigOperator is the constant to represent contents of Openshift_MachineConfigOperator.yaml file type OpenshiftMachineConfigOperator struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *OpenshiftMachineConfigOperator) Name() string { // Generate generates the actual files by this asset func (t *OpenshiftMachineConfigOperator) Generate(parents asset.Parents) error { - t.fileName = openshiftMachineConfigOperatorFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := openshiftMachineConfigOperatorFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/05-openshift-cluster-api-namespace.go b/pkg/asset/templates/content/bootkube/05-openshift-cluster-api-namespace.go index ff83d97094f..117245e5f11 100644 --- a/pkg/asset/templates/content/bootkube/05-openshift-cluster-api-namespace.go +++ b/pkg/asset/templates/content/bootkube/05-openshift-cluster-api-namespace.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*OpenshiftClusterAPINamespace)(nil) // OpenshiftClusterAPINamespace is the constant to represent contents of Openshift_ClusterApiNamespace.yaml file type OpenshiftClusterAPINamespace struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *OpenshiftClusterAPINamespace) Name() string { // Generate generates the actual files by this asset func (t *OpenshiftClusterAPINamespace) Generate(parents asset.Parents) error { - t.fileName = openshiftClusterAPINamespaceFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := openshiftClusterAPINamespaceFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/09-openshift-service-cert-signer-namespace.go b/pkg/asset/templates/content/bootkube/09-openshift-service-cert-signer-namespace.go index a5d9650846b..f5941e923f6 100644 --- a/pkg/asset/templates/content/bootkube/09-openshift-service-cert-signer-namespace.go +++ b/pkg/asset/templates/content/bootkube/09-openshift-service-cert-signer-namespace.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*OpenshiftServiceCertSignerNamespace)(nil) // OpenshiftServiceCertSignerNamespace is the constant to represent the contents of 09-openshift-service-signer-namespace.yaml type OpenshiftServiceCertSignerNamespace struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *OpenshiftServiceCertSignerNamespace) Name() string { // Generate generates the actual files by this asset func (t *OpenshiftServiceCertSignerNamespace) Generate(parents asset.Parents) error { - t.fileName = openshiftServiceCertSignerNamespaceFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := openshiftServiceCertSignerNamespaceFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/cvo-overrides.go b/pkg/asset/templates/content/bootkube/cvo-overrides.go index f331ba39032..a65ba274cf5 100644 --- a/pkg/asset/templates/content/bootkube/cvo-overrides.go +++ b/pkg/asset/templates/content/bootkube/cvo-overrides.go @@ -19,7 +19,6 @@ var _ asset.WritableAsset = (*CVOOverrides)(nil) // with resources already owned by other operators. // This files can be dropped when the overrides list becomes empty. type CVOOverrides struct { - fileName string FileList []*asset.File } @@ -35,14 +34,14 @@ func (t *CVOOverrides) Name() string { // Generate generates the actual files by this asset func (t *CVOOverrides) Generate(parents asset.Parents) error { - t.fileName = cVOOverridesFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := cVOOverridesFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/etcd-service.go b/pkg/asset/templates/content/bootkube/etcd-service.go index 90fdcdae699..6200c04789e 100644 --- a/pkg/asset/templates/content/bootkube/etcd-service.go +++ b/pkg/asset/templates/content/bootkube/etcd-service.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*EtcdServiceKubeSystem)(nil) // EtcdServiceKubeSystem is the constant to represent contents of etcd-service.yaml file type EtcdServiceKubeSystem struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *EtcdServiceKubeSystem) Name() string { // Generate generates the actual files by this asset func (t *EtcdServiceKubeSystem) Generate(parents asset.Parents) error { - t.fileName = etcdServiceKubeSystemFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := etcdServiceKubeSystemFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/host-etcd-service-endpoints.go b/pkg/asset/templates/content/bootkube/host-etcd-service-endpoints.go index e7d56072bf0..30f9b663e19 100644 --- a/pkg/asset/templates/content/bootkube/host-etcd-service-endpoints.go +++ b/pkg/asset/templates/content/bootkube/host-etcd-service-endpoints.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*HostEtcdServiceEndpointsKubeSystem)(nil) // HostEtcdServiceEndpointsKubeSystem is the constant to represent contents of etcd-service-endpoints.yaml.template file. type HostEtcdServiceEndpointsKubeSystem struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *HostEtcdServiceEndpointsKubeSystem) Name() string { // Generate generates the actual files by this asset func (t *HostEtcdServiceEndpointsKubeSystem) Generate(parents asset.Parents) error { - t.fileName = hostEtcdServiceEndpointsKubeSystemFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := hostEtcdServiceEndpointsKubeSystemFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/host-etcd-service.go b/pkg/asset/templates/content/bootkube/host-etcd-service.go index 95b3eac7af7..a1840f62c2b 100644 --- a/pkg/asset/templates/content/bootkube/host-etcd-service.go +++ b/pkg/asset/templates/content/bootkube/host-etcd-service.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*HostEtcdServiceKubeSystem)(nil) // HostEtcdServiceKubeSystem is the constant to represent contents of etcd-service.yaml file type HostEtcdServiceKubeSystem struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *HostEtcdServiceKubeSystem) Name() string { // Generate generates the actual files by this asset func (t *HostEtcdServiceKubeSystem) Generate(parents asset.Parents) error { - t.fileName = hostEtcdServiceKubeSystemFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := hostEtcdServiceKubeSystemFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/kube-cloud-config.go b/pkg/asset/templates/content/bootkube/kube-cloud-config.go index 1b5e649e486..59c097a5191 100644 --- a/pkg/asset/templates/content/bootkube/kube-cloud-config.go +++ b/pkg/asset/templates/content/bootkube/kube-cloud-config.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*KubeCloudConfig)(nil) // KubeCloudConfig is the constant to represent contents of kube_cloudconfig.yaml file type KubeCloudConfig struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *KubeCloudConfig) Name() string { // Generate generates the actual files by this asset func (t *KubeCloudConfig) Generate(parents asset.Parents) error { - t.fileName = kubeCloudConfigFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := kubeCloudConfigFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/kube-system-configmap-etcd-serving-ca.go b/pkg/asset/templates/content/bootkube/kube-system-configmap-etcd-serving-ca.go index 0d152ddba95..b578a0663b2 100644 --- a/pkg/asset/templates/content/bootkube/kube-system-configmap-etcd-serving-ca.go +++ b/pkg/asset/templates/content/bootkube/kube-system-configmap-etcd-serving-ca.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*KubeSystemConfigmapEtcdServingCA)(nil) // KubeSystemConfigmapEtcdServingCA is the constant to represent contents of kube-system-configmap-etcd-serving-ca.yaml.template file. type KubeSystemConfigmapEtcdServingCA struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *KubeSystemConfigmapEtcdServingCA) Name() string { // Generate generates the actual files by this asset func (t *KubeSystemConfigmapEtcdServingCA) Generate(parents asset.Parents) error { - t.fileName = kubeSystemConfigmapEtcdServingCAFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := kubeSystemConfigmapEtcdServingCAFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/kube-system-configmap-root-ca.go b/pkg/asset/templates/content/bootkube/kube-system-configmap-root-ca.go index 99a5b70a751..765961c36c8 100644 --- a/pkg/asset/templates/content/bootkube/kube-system-configmap-root-ca.go +++ b/pkg/asset/templates/content/bootkube/kube-system-configmap-root-ca.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*KubeSystemConfigmapRootCA)(nil) // KubeSystemConfigmapRootCA is the constant to represent contents of kube-system-configmap-root-ca.yaml.template file. type KubeSystemConfigmapRootCA struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *KubeSystemConfigmapRootCA) Name() string { // Generate generates the actual files by this asset func (t *KubeSystemConfigmapRootCA) Generate(parents asset.Parents) error { - t.fileName = kubeSystemConfigmapRootCAFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := kubeSystemConfigmapRootCAFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/kube-system-secret-etcd-client.go b/pkg/asset/templates/content/bootkube/kube-system-secret-etcd-client.go index 3ba3af00f4e..994d4b92f78 100644 --- a/pkg/asset/templates/content/bootkube/kube-system-secret-etcd-client.go +++ b/pkg/asset/templates/content/bootkube/kube-system-secret-etcd-client.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*KubeSystemSecretEtcdClient)(nil) // KubeSystemSecretEtcdClient is the constant to represent contents of kube-system-secret-etcd-client.yaml.template file. type KubeSystemSecretEtcdClient struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *KubeSystemSecretEtcdClient) Name() string { // Generate generates the actual files by this asset func (t *KubeSystemSecretEtcdClient) Generate(parents asset.Parents) error { - t.fileName = kubeSystemSecretEtcdClientFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := kubeSystemSecretEtcdClientFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/machine-config-server-tls-secret.go b/pkg/asset/templates/content/bootkube/machine-config-server-tls-secret.go index f5403bbdd6e..2246609fe4c 100644 --- a/pkg/asset/templates/content/bootkube/machine-config-server-tls-secret.go +++ b/pkg/asset/templates/content/bootkube/machine-config-server-tls-secret.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*MachineConfigServerTLSSecret)(nil) // MachineConfigServerTLSSecret is the constant to represent contents of machine_configservertlssecret.yaml.template file type MachineConfigServerTLSSecret struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *MachineConfigServerTLSSecret) Name() string { // Generate generates the actual files by this asset func (t *MachineConfigServerTLSSecret) Generate(parents asset.Parents) error { - t.fileName = machineConfigServerTLSSecretFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := machineConfigServerTLSSecretFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/openshift-service-cert-signer-ca-secret.go b/pkg/asset/templates/content/bootkube/openshift-service-cert-signer-ca-secret.go index ac84b473ddf..ea412ce1cd5 100644 --- a/pkg/asset/templates/content/bootkube/openshift-service-cert-signer-ca-secret.go +++ b/pkg/asset/templates/content/bootkube/openshift-service-cert-signer-ca-secret.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*OpenshiftServiceCertSignerSecret)(nil) // OpenshiftServiceCertSignerSecret is the constant to represent the contents of openshift-service-signer-secret.yaml.template type OpenshiftServiceCertSignerSecret struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *OpenshiftServiceCertSignerSecret) Name() string { // Generate generates the actual files by this asset func (t *OpenshiftServiceCertSignerSecret) Generate(parents asset.Parents) error { - t.fileName = openshiftServiceCertSignerSecretFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := openshiftServiceCertSignerSecretFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/bootkube/pull.go b/pkg/asset/templates/content/bootkube/pull.go index 02c5701bc2c..ba6627b032e 100644 --- a/pkg/asset/templates/content/bootkube/pull.go +++ b/pkg/asset/templates/content/bootkube/pull.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*Pull)(nil) // Pull is the constant to represent contents of pull.yaml.template file type Pull struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *Pull) Name() string { // Generate generates the actual files by this asset func (t *Pull) Generate(parents asset.Parents) error { - t.fileName = pullFileName - data, err := content.GetBootkubeTemplate(t.fileName) + fileName := pullFileName + data, err := content.GetBootkubeTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/openshift/binding-discovery.go b/pkg/asset/templates/content/openshift/binding-discovery.go index 1a5efcf2ccc..d6c8941bd12 100644 --- a/pkg/asset/templates/content/openshift/binding-discovery.go +++ b/pkg/asset/templates/content/openshift/binding-discovery.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*BindingDiscovery)(nil) // BindingDiscovery is the variable/constant representing the contents of the respective file type BindingDiscovery struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *BindingDiscovery) Name() string { // Generate generates the actual files by this asset func (t *BindingDiscovery) Generate(parents asset.Parents) error { - t.fileName = bindingDiscoveryFileName - data, err := content.GetOpenshiftTemplate(t.fileName) + fileName := bindingDiscoveryFileName + data, err := content.GetOpenshiftTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/openshift/cloud-creds-secret.go b/pkg/asset/templates/content/openshift/cloud-creds-secret.go index 0fde33ccfad..f2f661137b3 100644 --- a/pkg/asset/templates/content/openshift/cloud-creds-secret.go +++ b/pkg/asset/templates/content/openshift/cloud-creds-secret.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*CloudCredsSecret)(nil) // CloudCredsSecret is the constant to represent contents of corresponding yaml file type CloudCredsSecret struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *CloudCredsSecret) Name() string { // Generate generates the actual files by this asset func (t *CloudCredsSecret) Generate(parents asset.Parents) error { - t.fileName = cloudCredsSecretFileName - data, err := content.GetOpenshiftTemplate(t.fileName) + fileName := cloudCredsSecretFileName + data, err := content.GetOpenshiftTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/openshift/cluster-infrastructure-crd.go b/pkg/asset/templates/content/openshift/cluster-infrastructure-crd.go index 0c6ea11faab..ce414f8daa6 100644 --- a/pkg/asset/templates/content/openshift/cluster-infrastructure-crd.go +++ b/pkg/asset/templates/content/openshift/cluster-infrastructure-crd.go @@ -9,7 +9,7 @@ import ( ) const ( - infraCRDfilename = "cluster-infrastructure-crd.yaml" + infraCRDFilename = "cluster-infrastructure-crd.yaml" ) var _ asset.WritableAsset = (*InfrastructureCRD)(nil) @@ -17,7 +17,6 @@ var _ asset.WritableAsset = (*InfrastructureCRD)(nil) // InfrastructureCRD is the custom resource definition for the openshift/api // Infrastructure type. type InfrastructureCRD struct { - fileName string FileList []*asset.File } @@ -33,14 +32,13 @@ func (t *InfrastructureCRD) Name() string { // Generate generates the actual files by this asset func (t *InfrastructureCRD) Generate(parents asset.Parents) error { - t.fileName = infraCRDfilename - data, err := content.GetOpenshiftTemplate(t.fileName) + data, err := content.GetOpenshiftTemplate(infraCRDFilename) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, infraCRDFilename), Data: []byte(data), }, } @@ -54,7 +52,7 @@ func (t *InfrastructureCRD) Files() []*asset.File { // Load returns the asset from disk. func (t *InfrastructureCRD) Load(f asset.FileFetcher) (bool, error) { - file, err := f.FetchByName(filepath.Join(content.TemplateDir, infraCRDfilename)) + file, err := f.FetchByName(filepath.Join(content.TemplateDir, infraCRDFilename)) if err != nil { if os.IsNotExist(err) { return false, nil diff --git a/pkg/asset/templates/content/openshift/kubeadmin-password-secret.go b/pkg/asset/templates/content/openshift/kubeadmin-password-secret.go index aeedb196826..a132823fd12 100644 --- a/pkg/asset/templates/content/openshift/kubeadmin-password-secret.go +++ b/pkg/asset/templates/content/openshift/kubeadmin-password-secret.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*KubeadminPasswordSecret)(nil) // KubeadminPasswordSecret is the constant to represent contents of kubeadmin-password-secret.yaml.template file type KubeadminPasswordSecret struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *KubeadminPasswordSecret) Name() string { // Generate generates the actual files by this asset func (t *KubeadminPasswordSecret) Generate(parents asset.Parents) error { - t.fileName = kubeadminPasswordSecretFileName - data, err := content.GetOpenshiftTemplate(t.fileName) + fileName := kubeadminPasswordSecretFileName + data, err := content.GetOpenshiftTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } diff --git a/pkg/asset/templates/content/openshift/role-cloud-creds-secret-reader.go b/pkg/asset/templates/content/openshift/role-cloud-creds-secret-reader.go index 127b0414307..fbd35e31d36 100644 --- a/pkg/asset/templates/content/openshift/role-cloud-creds-secret-reader.go +++ b/pkg/asset/templates/content/openshift/role-cloud-creds-secret-reader.go @@ -16,7 +16,6 @@ var _ asset.WritableAsset = (*RoleCloudCredsSecretReader)(nil) // RoleCloudCredsSecretReader is the variable to represent contents of corresponding file type RoleCloudCredsSecretReader struct { - fileName string FileList []*asset.File } @@ -32,14 +31,14 @@ func (t *RoleCloudCredsSecretReader) Name() string { // Generate generates the actual files by this asset func (t *RoleCloudCredsSecretReader) Generate(parents asset.Parents) error { - t.fileName = roleCloudCredsSecretReaderFileName - data, err := content.GetOpenshiftTemplate(t.fileName) + fileName := roleCloudCredsSecretReaderFileName + data, err := content.GetOpenshiftTemplate(fileName) if err != nil { return err } t.FileList = []*asset.File{ { - Filename: filepath.Join(content.TemplateDir, t.fileName), + Filename: filepath.Join(content.TemplateDir, fileName), Data: []byte(data), }, } From a7fddff915b99ad67c67df99de74859e06c310e9 Mon Sep 17 00:00:00 2001 From: staebler Date: Wed, 12 Dec 2018 15:25:58 -0500 Subject: [PATCH 3/7] assets/templates: remove common templates asset The Common Templates asset is not truly an asset. It does not own any on-disk files, and no other assets depend upon it. The problem with the Common Templates asset is that it writes files to disk as a targeted asset but then does not load any assets back in. So a test that verifies that the loaded-in asset matches the written asset fails. Changes for https://jira.coreos.com/browse/CORS-940 --- cmd/openshift-install/create.go | 2 +- pkg/asset/templates/templates.go | 139 +++++-------------------------- 2 files changed, 24 insertions(+), 117 deletions(-) diff --git a/cmd/openshift-install/create.go b/cmd/openshift-install/create.go index e7277eded91..c58946fed2c 100644 --- a/cmd/openshift-install/create.go +++ b/cmd/openshift-install/create.go @@ -76,7 +76,7 @@ var ( Short: "Generates the unrendered Kubernetes manifest templates", Long: "", }, - assets: []asset.WritableAsset{&templates.Templates{}}, + assets: templates.Templates, } ignitionConfigsTarget = target{ diff --git a/pkg/asset/templates/templates.go b/pkg/asset/templates/templates.go index 82f9eaccd99..7821855b8b5 100644 --- a/pkg/asset/templates/templates.go +++ b/pkg/asset/templates/templates.go @@ -7,120 +7,27 @@ import ( "github.com/openshift/installer/pkg/asset/templates/content/openshift" ) -var _ asset.WritableAsset = (*Templates)(nil) - -// Templates generates the dependent unrendered template files -type Templates struct { - FileList []*asset.File -} - -// Name returns a human friendly name for the templates asset -func (m *Templates) Name() string { - return "Common Templates" -} - -// Dependencies returns all of the dependencies directly needed by a -// Templates asset. -func (m *Templates) Dependencies() []asset.Asset { - return []asset.Asset{ - &bootkube.KubeCloudConfig{}, - &bootkube.MachineConfigServerTLSSecret{}, - &bootkube.OpenshiftServiceCertSignerSecret{}, - &bootkube.Pull{}, - &bootkube.CVOOverrides{}, - &bootkube.HostEtcdServiceEndpointsKubeSystem{}, - &bootkube.KubeSystemConfigmapEtcdServingCA{}, - &bootkube.KubeSystemConfigmapRootCA{}, - &bootkube.KubeSystemSecretEtcdClient{}, - &bootkube.OpenshiftMachineConfigOperator{}, - &bootkube.OpenshiftClusterAPINamespace{}, - &bootkube.OpenshiftServiceCertSignerNamespace{}, - &bootkube.EtcdServiceKubeSystem{}, - &bootkube.HostEtcdServiceKubeSystem{}, - &openshift.BindingDiscovery{}, - &openshift.CloudCredsSecret{}, - &openshift.KubeadminPasswordSecret{}, - &openshift.RoleCloudCredsSecretReader{}, - &openshift.InfrastructureCRD{}, - &openshift.NetworkCRDs{}, - } -} - -// Generate generates the respective operator config.yml files -func (m *Templates) Generate(dependencies asset.Parents) error { - kubeCloudConfig := &bootkube.KubeCloudConfig{} - machineConfigServerTLSSecret := &bootkube.MachineConfigServerTLSSecret{} - openshiftServiceCertSignerSecret := &bootkube.OpenshiftServiceCertSignerSecret{} - pull := &bootkube.Pull{} - cVOOverrides := &bootkube.CVOOverrides{} - hostEtcdServiceEndpointsKubeSystem := &bootkube.HostEtcdServiceEndpointsKubeSystem{} - kubeSystemConfigmapEtcdServingCA := &bootkube.KubeSystemConfigmapEtcdServingCA{} - kubeSystemConfigmapRootCA := &bootkube.KubeSystemConfigmapRootCA{} - kubeSystemSecretEtcdClient := &bootkube.KubeSystemSecretEtcdClient{} - openshiftMachineConfigOperator := &bootkube.OpenshiftMachineConfigOperator{} - openshiftClusterAPINamespace := &bootkube.OpenshiftClusterAPINamespace{} - openshiftServiceCertSignerNamespace := &bootkube.OpenshiftServiceCertSignerNamespace{} - etcdServiceKubeSystem := &bootkube.EtcdServiceKubeSystem{} - hostEtcdServiceKubeSystem := &bootkube.HostEtcdServiceKubeSystem{} - - bindingDiscovery := &openshift.BindingDiscovery{} - cloudCredsSecret := &openshift.CloudCredsSecret{} - kubeadminPasswordSecret := &openshift.KubeadminPasswordSecret{} - roleCloudCredsSecretReader := &openshift.RoleCloudCredsSecretReader{} - infrastructure := &openshift.InfrastructureCRD{} - - dependencies.Get( - kubeCloudConfig, - machineConfigServerTLSSecret, - openshiftServiceCertSignerSecret, - pull, - cVOOverrides, - hostEtcdServiceEndpointsKubeSystem, - kubeSystemConfigmapEtcdServingCA, - kubeSystemConfigmapRootCA, - kubeSystemSecretEtcdClient, - openshiftMachineConfigOperator, - openshiftClusterAPINamespace, - openshiftServiceCertSignerNamespace, - etcdServiceKubeSystem, - hostEtcdServiceKubeSystem, - bindingDiscovery, - cloudCredsSecret, - kubeadminPasswordSecret, - roleCloudCredsSecretReader, - infrastructure) - - m.FileList = []*asset.File{} - m.FileList = append(m.FileList, kubeCloudConfig.Files()...) - m.FileList = append(m.FileList, machineConfigServerTLSSecret.Files()...) - m.FileList = append(m.FileList, openshiftServiceCertSignerSecret.Files()...) - m.FileList = append(m.FileList, pull.Files()...) - m.FileList = append(m.FileList, cVOOverrides.Files()...) - m.FileList = append(m.FileList, hostEtcdServiceEndpointsKubeSystem.Files()...) - m.FileList = append(m.FileList, kubeSystemConfigmapEtcdServingCA.Files()...) - m.FileList = append(m.FileList, kubeSystemConfigmapRootCA.Files()...) - m.FileList = append(m.FileList, kubeSystemSecretEtcdClient.Files()...) - m.FileList = append(m.FileList, openshiftMachineConfigOperator.Files()...) - m.FileList = append(m.FileList, openshiftClusterAPINamespace.Files()...) - m.FileList = append(m.FileList, openshiftServiceCertSignerNamespace.Files()...) - m.FileList = append(m.FileList, etcdServiceKubeSystem.Files()...) - m.FileList = append(m.FileList, hostEtcdServiceKubeSystem.Files()...) - - m.FileList = append(m.FileList, bindingDiscovery.Files()...) - m.FileList = append(m.FileList, cloudCredsSecret.Files()...) - m.FileList = append(m.FileList, kubeadminPasswordSecret.Files()...) - m.FileList = append(m.FileList, roleCloudCredsSecretReader.Files()...) - m.FileList = append(m.FileList, infrastructure.Files()...) - - return nil -} - -// Files returns the files generated by the asset. -func (m *Templates) Files() []*asset.File { - return m.FileList -} - -// Load returns the manifests asset from disk. -func (m *Templates) Load(f asset.FileFetcher) (bool, error) { - return false, nil +// Templates are the targeted assets for generating the dependent unrendered +// template files. +var Templates = []asset.WritableAsset{ + &bootkube.KubeCloudConfig{}, + &bootkube.MachineConfigServerTLSSecret{}, + &bootkube.OpenshiftServiceCertSignerSecret{}, + &bootkube.Pull{}, + &bootkube.CVOOverrides{}, + &bootkube.HostEtcdServiceEndpointsKubeSystem{}, + &bootkube.KubeSystemConfigmapEtcdServingCA{}, + &bootkube.KubeSystemConfigmapRootCA{}, + &bootkube.KubeSystemSecretEtcdClient{}, + &bootkube.OpenshiftMachineConfigOperator{}, + &bootkube.OpenshiftClusterAPINamespace{}, + &bootkube.OpenshiftServiceCertSignerNamespace{}, + &bootkube.EtcdServiceKubeSystem{}, + &bootkube.HostEtcdServiceKubeSystem{}, + &openshift.BindingDiscovery{}, + &openshift.CloudCredsSecret{}, + &openshift.KubeadminPasswordSecret{}, + &openshift.RoleCloudCredsSecretReader{}, + &openshift.InfrastructureCRD{}, + &openshift.NetworkCRDs{}, } From c6f6449e2e77b8eb897a23276e8d27f3343ba1bf Mon Sep 17 00:00:00 2001 From: staebler Date: Wed, 12 Dec 2018 15:24:00 -0500 Subject: [PATCH 4/7] assets: move asset store impl to own package Tests are being created that use the asset store implementation to validate restoring assets from the state file and disk. The tests are much easier when they have access to the private fields of the asset store. Rather than exposing more public fields and methods on the store, the tests will be placed in the same package as the store. Unfortunately, this creates cyclic imports between the base asset package and the various packages containing the targeted assets. To rectify this, the concrete asset store implementation has been moved to a new pkg/asset/store package. Changes for https://jira.coreos.com/browse/CORS-940 --- cmd/openshift-install/create.go | 3 +- cmd/openshift-install/destroy.go | 4 +- pkg/asset/asset.go | 4 +- pkg/asset/filefetcher.go | 48 --- pkg/asset/store.go | 350 --------------------- pkg/asset/store/data | 1 + pkg/asset/store/filefetcher.go | 51 ++++ pkg/asset/{ => store}/filefetcher_test.go | 16 +- pkg/asset/store/store.go | 357 ++++++++++++++++++++++ pkg/asset/{ => store}/store_test.go | 64 ++-- 10 files changed, 457 insertions(+), 441 deletions(-) create mode 120000 pkg/asset/store/data create mode 100644 pkg/asset/store/filefetcher.go rename pkg/asset/{ => store}/filefetcher_test.go (94%) create mode 100644 pkg/asset/store/store.go rename pkg/asset/{ => store}/store_test.go (82%) diff --git a/cmd/openshift-install/create.go b/cmd/openshift-install/create.go index c58946fed2c..c5349128525 100644 --- a/cmd/openshift-install/create.go +++ b/cmd/openshift-install/create.go @@ -32,6 +32,7 @@ import ( "github.com/openshift/installer/pkg/asset/installconfig" "github.com/openshift/installer/pkg/asset/kubeconfig" "github.com/openshift/installer/pkg/asset/manifests" + assetstore "github.com/openshift/installer/pkg/asset/store" "github.com/openshift/installer/pkg/asset/templates" "github.com/openshift/installer/pkg/asset/tls" destroybootstrap "github.com/openshift/installer/pkg/destroy/bootstrap" @@ -154,7 +155,7 @@ func newCreateCmd() *cobra.Command { func runTargetCmd(targets ...asset.WritableAsset) func(cmd *cobra.Command, args []string) { runner := func(directory string) error { - assetStore, err := asset.NewStore(directory) + assetStore, err := assetstore.NewStore(directory) if err != nil { return errors.Wrapf(err, "failed to create asset store") } diff --git a/cmd/openshift-install/destroy.go b/cmd/openshift-install/destroy.go index 002a6a04519..747c106850b 100644 --- a/cmd/openshift-install/destroy.go +++ b/cmd/openshift-install/destroy.go @@ -5,7 +5,7 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "github.com/openshift/installer/pkg/asset" + assetstore "github.com/openshift/installer/pkg/asset/store" "github.com/openshift/installer/pkg/destroy" "github.com/openshift/installer/pkg/destroy/bootstrap" _ "github.com/openshift/installer/pkg/destroy/libvirt" @@ -52,7 +52,7 @@ func runDestroyCmd(directory string) error { return errors.Wrap(err, "Failed to destroy cluster") } - store, err := asset.NewStore(directory) + store, err := assetstore.NewStore(directory) if err != nil { return errors.Wrapf(err, "failed to create asset store") } diff --git a/pkg/asset/asset.go b/pkg/asset/asset.go index b08652b0d2a..6fb4e05b8fe 100644 --- a/pkg/asset/asset.go +++ b/pkg/asset/asset.go @@ -58,9 +58,9 @@ func PersistToFile(asset WritableAsset, directory string) error { return nil } -// deleteAssetFromDisk removes all the files for asset from disk. +// DeleteAssetFromDisk removes all the files for asset from disk. // this is function is not safe for calling concurrently on the same directory. -func deleteAssetFromDisk(asset WritableAsset, directory string) error { +func DeleteAssetFromDisk(asset WritableAsset, directory string) error { logrus.Debugf("Purging asset %q from disk", asset.Name()) for _, f := range asset.Files() { path := filepath.Join(directory, f.Filename) diff --git a/pkg/asset/filefetcher.go b/pkg/asset/filefetcher.go index 2520caffffa..5aece890ea3 100644 --- a/pkg/asset/filefetcher.go +++ b/pkg/asset/filefetcher.go @@ -1,11 +1,5 @@ package asset -import ( - "io/ioutil" - "path/filepath" - "sort" -) - //go:generate mockgen -source=./filefetcher.go -destination=./mock/filefetcher_generated.go -package=mock // FileFetcher fetches the asset files from disk. @@ -15,45 +9,3 @@ type FileFetcher interface { // FetchByPattern returns the files whose name match the given glob. FetchByPattern(pattern string) ([]*File, error) } - -type fileFetcher struct { - directory string -} - -// FetchByName returns the file with the given name. -func (f *fileFetcher) FetchByName(name string) (*File, error) { - data, err := ioutil.ReadFile(filepath.Join(f.directory, name)) - if err != nil { - return nil, err - } - return &File{Filename: name, Data: data}, nil -} - -// FetchByPattern returns the files whose name match the given regexp. -func (f *fileFetcher) FetchByPattern(pattern string) (files []*File, err error) { - matches, err := filepath.Glob(filepath.Join(f.directory, pattern)) - if err != nil { - return nil, err - } - - files = make([]*File, 0, len(matches)) - for _, path := range matches { - data, err := ioutil.ReadFile(path) - if err != nil { - return nil, err - } - - filename, err := filepath.Rel(f.directory, path) - if err != nil { - return nil, err - } - - files = append(files, &File{ - Filename: filename, - Data: data, - }) - } - - sort.Slice(files, func(i, j int) bool { return files[i].Filename < files[j].Filename }) - return files, nil -} diff --git a/pkg/asset/store.go b/pkg/asset/store.go index 9557aefd640..867ec3f521b 100644 --- a/pkg/asset/store.go +++ b/pkg/asset/store.go @@ -1,20 +1,5 @@ package asset -import ( - "encoding/json" - "io/ioutil" - "os" - "path/filepath" - "reflect" - - "github.com/pkg/errors" - "github.com/sirupsen/logrus" -) - -const ( - stateFileName = ".openshift_install_state.json" -) - // Store is a store for the states of assets. type Store interface { // Fetch retrieves the state of the given asset, generating it and its @@ -29,338 +14,3 @@ type Store interface { // state file DestroyState() error } - -// assetSource indicates from where the asset was fetched -type assetSource int - -const ( - // unsourced indicates that the asset has not been fetched - unfetched assetSource = iota - // generatedSource indicates that the asset was generated - generatedSource - // onDiskSource indicates that the asset was fetched from disk - onDiskSource - // stateFileSource indicates that the asset was fetched from the state file - stateFileSource -) - -type assetState struct { - // asset is the asset. - // If the asset has not been fetched, then this will be nil. - asset Asset - // source is the source from which the asset was fetched - source assetSource - // anyParentsDirty is true if any of the parents of the asset are dirty - anyParentsDirty bool - // presentOnDisk is true if the asset in on-disk. This is set whether the - // asset is sourced from on-disk or not. It is used in purging consumed assets. - presentOnDisk bool -} - -// StoreImpl is the implementation of Store. -type StoreImpl struct { - directory string - assets map[reflect.Type]*assetState - stateFileAssets map[string]json.RawMessage - fileFetcher FileFetcher -} - -// NewStore returns an asset store that implements the Store interface. -func NewStore(dir string) (Store, error) { - store := &StoreImpl{ - directory: dir, - fileFetcher: &fileFetcher{directory: dir}, - assets: map[reflect.Type]*assetState{}, - } - - if err := store.loadStateFile(); err != nil { - return nil, err - } - return store, nil -} - -// Fetch retrieves the state of the given asset, generating it and its -// dependencies if necessary. -func (s *StoreImpl) Fetch(asset Asset) error { - if err := s.fetch(asset, ""); err != nil { - return err - } - if err := s.saveStateFile(); err != nil { - return errors.Wrapf(err, "failed to save state") - } - if wa, ok := asset.(WritableAsset); ok { - return errors.Wrapf(s.purge(wa), "failed to purge asset") - } - return nil -} - -// Destroy removes the asset from all its internal state and also from -// disk if possible. -func (s *StoreImpl) Destroy(asset Asset) error { - if sa, ok := s.assets[reflect.TypeOf(asset)]; ok { - reflect.ValueOf(asset).Elem().Set(reflect.ValueOf(sa.asset).Elem()) - } else if s.isAssetInState(asset) { - if err := s.loadAssetFromState(asset); err != nil { - return err - } - } else { - // nothing to do - return nil - } - - if wa, ok := asset.(WritableAsset); ok { - if err := deleteAssetFromDisk(wa, s.directory); err != nil { - return err - } - } - - delete(s.assets, reflect.TypeOf(asset)) - delete(s.stateFileAssets, reflect.TypeOf(asset).String()) - return s.saveStateFile() -} - -// DestroyState removes the state file from disk -func (s *StoreImpl) DestroyState() error { - s.stateFileAssets = nil - path := filepath.Join(s.directory, stateFileName) - err := os.Remove(path) - if err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } - return nil -} - -// loadStateFile retrieves the state from the state file present in the given directory -// and returns the assets map -func (s *StoreImpl) loadStateFile() error { - path := filepath.Join(s.directory, stateFileName) - assets := map[string]json.RawMessage{} - data, err := ioutil.ReadFile(path) - if err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } - err = json.Unmarshal(data, &assets) - if err != nil { - return errors.Wrapf(err, "failed to unmarshal state file %q", path) - } - s.stateFileAssets = assets - return nil -} - -// loadAssetFromState renders the asset object arguments from the state file contents. -func (s *StoreImpl) loadAssetFromState(asset Asset) error { - bytes, ok := s.stateFileAssets[reflect.TypeOf(asset).String()] - if !ok { - return errors.Errorf("asset %q is not found in the state file", asset.Name()) - } - return json.Unmarshal(bytes, asset) -} - -// isAssetInState tests whether the asset is in the state file. -func (s *StoreImpl) isAssetInState(asset Asset) bool { - _, ok := s.stateFileAssets[reflect.TypeOf(asset).String()] - return ok -} - -// saveStateFile dumps the entire state map into a file -func (s *StoreImpl) saveStateFile() error { - if s.stateFileAssets == nil { - s.stateFileAssets = map[string]json.RawMessage{} - } - for k, v := range s.assets { - if v.source == unfetched { - continue - } - data, err := json.MarshalIndent(v.asset, "", " ") - if err != nil { - return err - } - s.stateFileAssets[k.String()] = json.RawMessage(data) - } - data, err := json.MarshalIndent(s.stateFileAssets, "", " ") - if err != nil { - return err - } - - path := filepath.Join(s.directory, stateFileName) - if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { - return err - } - if err := ioutil.WriteFile(path, data, 0644); err != nil { - return err - } - return nil -} - -// fetch populates the given asset, generating it and its dependencies if -// necessary, and returns whether or not the asset had to be regenerated and -// any errors. -func (s *StoreImpl) fetch(asset Asset, indent string) error { - logrus.Debugf("%sFetching %q...", indent, asset.Name()) - - assetState, ok := s.assets[reflect.TypeOf(asset)] - if !ok { - if _, err := s.load(asset, ""); err != nil { - return err - } - assetState = s.assets[reflect.TypeOf(asset)] - } - - // Return immediately if the asset has been fetched before, - // this is because we are doing a depth-first-search, it's guaranteed - // that we always fetch the parent before children, so we don't need - // to worry about invalidating anything in the cache. - if assetState.source != unfetched { - logrus.Debugf("%sReusing previously-fetched %q", indent, asset.Name()) - reflect.ValueOf(asset).Elem().Set(reflect.ValueOf(assetState.asset).Elem()) - return nil - } - - // Re-generate the asset - dependencies := asset.Dependencies() - parents := make(Parents, len(dependencies)) - for _, d := range dependencies { - if err := s.fetch(d, increaseIndent(indent)); err != nil { - return errors.Wrapf(err, "failed to fetch dependency of %q", asset.Name()) - } - parents.Add(d) - } - logrus.Debugf("%sGenerating %q...", indent, asset.Name()) - if err := asset.Generate(parents); err != nil { - return errors.Wrapf(err, "failed to generate asset %q", asset.Name()) - } - assetState.asset = asset - assetState.source = generatedSource - return nil -} - -// load loads the asset and all of its ancestors from on-disk and the state file. -func (s *StoreImpl) load(asset Asset, indent string) (*assetState, error) { - logrus.Debugf("%sLoading %q...", indent, asset.Name()) - - // Stop descent if the asset has already been loaded. - if state, ok := s.assets[reflect.TypeOf(asset)]; ok { - return state, nil - } - - // Load dependencies from on-disk. - anyParentsDirty := false - for _, d := range asset.Dependencies() { - state, err := s.load(d, increaseIndent(indent)) - if err != nil { - return nil, err - } - if state.anyParentsDirty || state.source == onDiskSource { - anyParentsDirty = true - } - } - - // Try to load from on-disk. - var ( - onDiskAsset WritableAsset - foundOnDisk bool - ) - if _, isWritable := asset.(WritableAsset); isWritable { - onDiskAsset = reflect.New(reflect.TypeOf(asset).Elem()).Interface().(WritableAsset) - var err error - foundOnDisk, err = onDiskAsset.Load(s.fileFetcher) - if err != nil { - return nil, errors.Wrapf(err, "failed to load asset %q", asset.Name()) - } - } - - // Try to load from state file. - var ( - stateFileAsset Asset - foundInStateFile bool - onDiskMatchesStateFile bool - ) - // Do not need to bother with loading from state file if any of the parents - // are dirty because the asset must be re-generated in this case. - if !anyParentsDirty { - foundInStateFile = s.isAssetInState(asset) - if foundInStateFile { - stateFileAsset = reflect.New(reflect.TypeOf(asset).Elem()).Interface().(Asset) - if err := s.loadAssetFromState(stateFileAsset); err != nil { - return nil, errors.Wrapf(err, "failed to load asset %q from state file", asset.Name()) - } - } - - if foundOnDisk && foundInStateFile { - logrus.Debugf("%sLoading %q from both state file and target directory", indent, asset.Name()) - - // If the on-disk asset is the same as the one in the state file, there - // is no need to consider the one on disk and to mark the asset dirty. - onDiskMatchesStateFile = reflect.DeepEqual(onDiskAsset, stateFileAsset) - if onDiskMatchesStateFile { - logrus.Debugf("%sOn-disk %q matches asset in state file", indent, asset.Name()) - } - } - } - - var ( - assetToStore Asset - source assetSource - ) - switch { - // A parent is dirty. The asset must be re-generated. - case anyParentsDirty: - if foundOnDisk { - logrus.Warningf("%sDiscarding the %q that was provided in the target directory because its dependencies are dirty and it needs to be regenerated", indent, asset.Name()) - } - source = unfetched - // The asset is on disk and that differs from what is in the source file. - // The asset is sourced from on disk. - case foundOnDisk && !onDiskMatchesStateFile: - logrus.Debugf("%sUsing %q loaded from target directory", indent, asset.Name()) - assetToStore = onDiskAsset - source = onDiskSource - // The asset is in the state file. The asset is sourced from state file. - case foundInStateFile: - logrus.Debugf("%sUsing %q loaded from state file", indent, asset.Name()) - assetToStore = stateFileAsset - source = stateFileSource - // There is no existing source for the asset. The asset will be generated. - default: - source = unfetched - } - - state := &assetState{ - asset: assetToStore, - source: source, - anyParentsDirty: anyParentsDirty, - presentOnDisk: foundOnDisk, - } - s.assets[reflect.TypeOf(asset)] = state - return state, nil -} - -// purge deletes the on-disk assets that are consumed already. -// E.g., install-config.yaml will be deleted after fetching 'manifests'. -// The target asset is excluded. -func (s *StoreImpl) purge(excluded WritableAsset) error { - for _, assetState := range s.assets { - if !assetState.presentOnDisk { - continue - } - if reflect.TypeOf(assetState.asset) == reflect.TypeOf(excluded) { - continue - } - logrus.Infof("Consuming %q from target directory", assetState.asset.Name()) - if err := deleteAssetFromDisk(assetState.asset.(WritableAsset), s.directory); err != nil { - return err - } - assetState.presentOnDisk = false - } - return nil -} - -func increaseIndent(indent string) string { - return indent + " " -} diff --git a/pkg/asset/store/data b/pkg/asset/store/data new file mode 120000 index 00000000000..45606f08b05 --- /dev/null +++ b/pkg/asset/store/data @@ -0,0 +1 @@ +../../../data/data \ No newline at end of file diff --git a/pkg/asset/store/filefetcher.go b/pkg/asset/store/filefetcher.go new file mode 100644 index 00000000000..03703ab1039 --- /dev/null +++ b/pkg/asset/store/filefetcher.go @@ -0,0 +1,51 @@ +package store + +import ( + "io/ioutil" + "path/filepath" + "sort" + + "github.com/openshift/installer/pkg/asset" +) + +type fileFetcher struct { + directory string +} + +// FetchByName returns the file with the given name. +func (f *fileFetcher) FetchByName(name string) (*asset.File, error) { + data, err := ioutil.ReadFile(filepath.Join(f.directory, name)) + if err != nil { + return nil, err + } + return &asset.File{Filename: name, Data: data}, nil +} + +// FetchByPattern returns the files whose name match the given regexp. +func (f *fileFetcher) FetchByPattern(pattern string) (files []*asset.File, err error) { + matches, err := filepath.Glob(filepath.Join(f.directory, pattern)) + if err != nil { + return nil, err + } + + files = make([]*asset.File, 0, len(matches)) + for _, path := range matches { + data, err := ioutil.ReadFile(path) + if err != nil { + return nil, err + } + + filename, err := filepath.Rel(f.directory, path) + if err != nil { + return nil, err + } + + files = append(files, &asset.File{ + Filename: filename, + Data: data, + }) + } + + sort.Slice(files, func(i, j int) bool { return files[i].Filename < files[j].Filename }) + return files, nil +} diff --git a/pkg/asset/filefetcher_test.go b/pkg/asset/store/filefetcher_test.go similarity index 94% rename from pkg/asset/filefetcher_test.go rename to pkg/asset/store/filefetcher_test.go index 43cb52b6c26..42af7928fba 100644 --- a/pkg/asset/filefetcher_test.go +++ b/pkg/asset/store/filefetcher_test.go @@ -1,4 +1,4 @@ -package asset +package store import ( "io/ioutil" @@ -7,6 +7,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "github.com/openshift/installer/pkg/asset" ) func TestFetchByName(t *testing.T) { @@ -14,7 +16,7 @@ func TestFetchByName(t *testing.T) { name string files map[string][]byte input string - expectFile *File + expectFile *asset.File }{ { name: "input doesn't match", @@ -26,7 +28,7 @@ func TestFetchByName(t *testing.T) { name: "with contents", files: map[string][]byte{"foo.bar": []byte("some data")}, input: "foo.bar", - expectFile: &File{ + expectFile: &asset.File{ Filename: "foo.bar", Data: []byte("some data"), }, @@ -35,7 +37,7 @@ func TestFetchByName(t *testing.T) { name: "match one file", files: map[string][]byte{"foo.bar": []byte("some data")}, input: "foo.bar", - expectFile: &File{ + expectFile: &asset.File{ Filename: "foo.bar", Data: []byte("some data"), }, @@ -110,11 +112,11 @@ func TestFetchByPattern(t *testing.T) { } tests := []struct { input string - expectFiles []*File + expectFiles []*asset.File }{ { input: "master-[0-9]*.ign", - expectFiles: []*File{ + expectFiles: []*asset.File{ { Filename: "master-0.ign", Data: []byte("some data 0"), @@ -147,7 +149,7 @@ func TestFetchByPattern(t *testing.T) { }, { input: filepath.Join("manifests", "*"), - expectFiles: []*File{ + expectFiles: []*asset.File{ { Filename: "manifests/0", Data: []byte("some data 11"), diff --git a/pkg/asset/store/store.go b/pkg/asset/store/store.go new file mode 100644 index 00000000000..ee966687b47 --- /dev/null +++ b/pkg/asset/store/store.go @@ -0,0 +1,357 @@ +package store + +import ( + "encoding/json" + "io/ioutil" + "os" + "path/filepath" + "reflect" + + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + + "github.com/openshift/installer/pkg/asset" +) + +const ( + stateFileName = ".openshift_install_state.json" +) + +// assetSource indicates from where the asset was fetched +type assetSource int + +const ( + // unsourced indicates that the asset has not been fetched + unfetched assetSource = iota + // generatedSource indicates that the asset was generated + generatedSource + // onDiskSource indicates that the asset was fetched from disk + onDiskSource + // stateFileSource indicates that the asset was fetched from the state file + stateFileSource +) + +type assetState struct { + // asset is the asset. + // If the asset has not been fetched, then this will be nil. + asset asset.Asset + // source is the source from which the asset was fetched + source assetSource + // anyParentsDirty is true if any of the parents of the asset are dirty + anyParentsDirty bool + // presentOnDisk is true if the asset in on-disk. This is set whether the + // asset is sourced from on-disk or not. It is used in purging consumed assets. + presentOnDisk bool +} + +// storeImpl is the implementation of Store. +type storeImpl struct { + directory string + assets map[reflect.Type]*assetState + stateFileAssets map[string]json.RawMessage + fileFetcher asset.FileFetcher +} + +// NewStore returns an asset store that implements the asset.Store interface. +func NewStore(dir string) (asset.Store, error) { + return newStore(dir) +} + +func newStore(dir string) (*storeImpl, error) { + store := &storeImpl{ + directory: dir, + fileFetcher: &fileFetcher{directory: dir}, + assets: map[reflect.Type]*assetState{}, + } + + if err := store.loadStateFile(); err != nil { + return nil, err + } + return store, nil +} + +// Fetch retrieves the state of the given asset, generating it and its +// dependencies if necessary. +func (s *storeImpl) Fetch(a asset.Asset) error { + if err := s.fetch(a, ""); err != nil { + return err + } + if err := s.saveStateFile(); err != nil { + return errors.Wrapf(err, "failed to save state") + } + if wa, ok := a.(asset.WritableAsset); ok { + return errors.Wrapf(s.purge(wa), "failed to purge asset") + } + return nil +} + +// Destroy removes the asset from all its internal state and also from +// disk if possible. +func (s *storeImpl) Destroy(a asset.Asset) error { + if sa, ok := s.assets[reflect.TypeOf(a)]; ok { + reflect.ValueOf(a).Elem().Set(reflect.ValueOf(sa.asset).Elem()) + } else if s.isAssetInState(a) { + if err := s.loadAssetFromState(a); err != nil { + return err + } + } else { + // nothing to do + return nil + } + + if wa, ok := a.(asset.WritableAsset); ok { + if err := asset.DeleteAssetFromDisk(wa, s.directory); err != nil { + return err + } + } + + delete(s.assets, reflect.TypeOf(a)) + delete(s.stateFileAssets, reflect.TypeOf(a).String()) + return s.saveStateFile() +} + +// DestroyState removes the state file from disk +func (s *storeImpl) DestroyState() error { + s.stateFileAssets = nil + path := filepath.Join(s.directory, stateFileName) + err := os.Remove(path) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + return nil +} + +// loadStateFile retrieves the state from the state file present in the given directory +// and returns the assets map +func (s *storeImpl) loadStateFile() error { + path := filepath.Join(s.directory, stateFileName) + assets := map[string]json.RawMessage{} + data, err := ioutil.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + err = json.Unmarshal(data, &assets) + if err != nil { + return errors.Wrapf(err, "failed to unmarshal state file %q", path) + } + s.stateFileAssets = assets + return nil +} + +// loadAssetFromState renders the asset object arguments from the state file contents. +func (s *storeImpl) loadAssetFromState(a asset.Asset) error { + bytes, ok := s.stateFileAssets[reflect.TypeOf(a).String()] + if !ok { + return errors.Errorf("asset %q is not found in the state file", a.Name()) + } + return json.Unmarshal(bytes, a) +} + +// isAssetInState tests whether the asset is in the state file. +func (s *storeImpl) isAssetInState(a asset.Asset) bool { + _, ok := s.stateFileAssets[reflect.TypeOf(a).String()] + return ok +} + +// saveStateFile dumps the entire state map into a file +func (s *storeImpl) saveStateFile() error { + if s.stateFileAssets == nil { + s.stateFileAssets = map[string]json.RawMessage{} + } + for k, v := range s.assets { + if v.source == unfetched { + continue + } + data, err := json.MarshalIndent(v.asset, "", " ") + if err != nil { + return err + } + s.stateFileAssets[k.String()] = json.RawMessage(data) + } + data, err := json.MarshalIndent(s.stateFileAssets, "", " ") + if err != nil { + return err + } + + path := filepath.Join(s.directory, stateFileName) + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + if err := ioutil.WriteFile(path, data, 0644); err != nil { + return err + } + return nil +} + +// fetch populates the given asset, generating it and its dependencies if +// necessary, and returns whether or not the asset had to be regenerated and +// any errors. +func (s *storeImpl) fetch(a asset.Asset, indent string) error { + logrus.Debugf("%sFetching %q...", indent, a.Name()) + + assetState, ok := s.assets[reflect.TypeOf(a)] + if !ok { + if _, err := s.load(a, ""); err != nil { + return err + } + assetState = s.assets[reflect.TypeOf(a)] + } + + // Return immediately if the asset has been fetched before, + // this is because we are doing a depth-first-search, it's guaranteed + // that we always fetch the parent before children, so we don't need + // to worry about invalidating anything in the cache. + if assetState.source != unfetched { + logrus.Debugf("%sReusing previously-fetched %q", indent, a.Name()) + reflect.ValueOf(a).Elem().Set(reflect.ValueOf(assetState.asset).Elem()) + return nil + } + + // Re-generate the asset + dependencies := a.Dependencies() + parents := make(asset.Parents, len(dependencies)) + for _, d := range dependencies { + if err := s.fetch(d, increaseIndent(indent)); err != nil { + return errors.Wrapf(err, "failed to fetch dependency of %q", a.Name()) + } + parents.Add(d) + } + logrus.Debugf("%sGenerating %q...", indent, a.Name()) + if err := a.Generate(parents); err != nil { + return errors.Wrapf(err, "failed to generate asset %q", a.Name()) + } + assetState.asset = a + assetState.source = generatedSource + return nil +} + +// load loads the asset and all of its ancestors from on-disk and the state file. +func (s *storeImpl) load(a asset.Asset, indent string) (*assetState, error) { + logrus.Debugf("%sLoading %q...", indent, a.Name()) + + // Stop descent if the asset has already been loaded. + if state, ok := s.assets[reflect.TypeOf(a)]; ok { + return state, nil + } + + // Load dependencies from on-disk. + anyParentsDirty := false + for _, d := range a.Dependencies() { + state, err := s.load(d, increaseIndent(indent)) + if err != nil { + return nil, err + } + if state.anyParentsDirty || state.source == onDiskSource { + anyParentsDirty = true + } + } + + // Try to load from on-disk. + var ( + onDiskAsset asset.WritableAsset + foundOnDisk bool + ) + if _, isWritable := a.(asset.WritableAsset); isWritable { + onDiskAsset = reflect.New(reflect.TypeOf(a).Elem()).Interface().(asset.WritableAsset) + var err error + foundOnDisk, err = onDiskAsset.Load(s.fileFetcher) + if err != nil { + return nil, errors.Wrapf(err, "failed to load asset %q", a.Name()) + } + } + + // Try to load from state file. + var ( + stateFileAsset asset.Asset + foundInStateFile bool + onDiskMatchesStateFile bool + ) + // Do not need to bother with loading from state file if any of the parents + // are dirty because the asset must be re-generated in this case. + if !anyParentsDirty { + foundInStateFile = s.isAssetInState(a) + if foundInStateFile { + stateFileAsset = reflect.New(reflect.TypeOf(a).Elem()).Interface().(asset.Asset) + if err := s.loadAssetFromState(stateFileAsset); err != nil { + return nil, errors.Wrapf(err, "failed to load asset %q from state file", a.Name()) + } + } + + if foundOnDisk && foundInStateFile { + logrus.Debugf("%sLoading %q from both state file and target directory", indent, a.Name()) + + // If the on-disk asset is the same as the one in the state file, there + // is no need to consider the one on disk and to mark the asset dirty. + onDiskMatchesStateFile = reflect.DeepEqual(onDiskAsset, stateFileAsset) + if onDiskMatchesStateFile { + logrus.Debugf("%sOn-disk %q matches asset in state file", indent, a.Name()) + } + } + } + + var ( + assetToStore asset.Asset + source assetSource + ) + switch { + // A parent is dirty. The asset must be re-generated. + case anyParentsDirty: + if foundOnDisk { + logrus.Warningf("%sDiscarding the %q that was provided in the target directory because its dependencies are dirty and it needs to be regenerated", indent, a.Name()) + } + source = unfetched + // The asset is on disk and that differs from what is in the source file. + // The asset is sourced from on disk. + case foundOnDisk && !onDiskMatchesStateFile: + logrus.Debugf("%sUsing %q loaded from target directory", indent, a.Name()) + assetToStore = onDiskAsset + source = onDiskSource + // The asset is in the state file. The asset is sourced from state file. + case foundInStateFile: + logrus.Debugf("%sUsing %q loaded from state file", indent, a.Name()) + assetToStore = stateFileAsset + source = stateFileSource + // There is no existing source for the asset. The asset will be generated. + default: + source = unfetched + } + + state := &assetState{ + asset: assetToStore, + source: source, + anyParentsDirty: anyParentsDirty, + presentOnDisk: foundOnDisk, + } + s.assets[reflect.TypeOf(a)] = state + return state, nil +} + +// purge deletes the on-disk assets that are consumed already. +// E.g., install-config.yaml will be deleted after fetching 'manifests'. +// The target asset is excluded. +func (s *storeImpl) purge(excluded asset.WritableAsset) error { + for _, assetState := range s.assets { + if !assetState.presentOnDisk { + continue + } + if reflect.TypeOf(assetState.asset) == reflect.TypeOf(excluded) { + continue + } + logrus.Infof("Consuming %q from target directory", assetState.asset.Name()) + if err := asset.DeleteAssetFromDisk(assetState.asset.(asset.WritableAsset), s.directory); err != nil { + return err + } + assetState.presentOnDisk = false + } + return nil +} + +func increaseIndent(indent string) string { + return indent + " " +} diff --git a/pkg/asset/store_test.go b/pkg/asset/store/store_test.go similarity index 82% rename from pkg/asset/store_test.go rename to pkg/asset/store/store_test.go index 8c13d44b2d8..7b99c1f33bd 100644 --- a/pkg/asset/store_test.go +++ b/pkg/asset/store/store_test.go @@ -1,4 +1,4 @@ -package asset +package store import ( "io/ioutil" @@ -7,6 +7,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "github.com/openshift/installer/pkg/asset" ) var ( @@ -14,30 +16,30 @@ var ( // asset store creates new assets by type, so the tests cannot store behavior // state in the assets themselves. generationLog []string - dependencies map[reflect.Type][]Asset + dependencies map[reflect.Type][]asset.Asset onDiskAssets map[reflect.Type]bool ) func clearAssetBehaviors() { generationLog = []string{} - dependencies = map[reflect.Type][]Asset{} + dependencies = map[reflect.Type][]asset.Asset{} onDiskAssets = map[reflect.Type]bool{} } -func dependenciesTestStoreAsset(a Asset) []Asset { +func dependenciesTestStoreAsset(a asset.Asset) []asset.Asset { return dependencies[reflect.TypeOf(a)] } -func generateTestStoreAsset(a Asset) error { +func generateTestStoreAsset(a asset.Asset) error { generationLog = append(generationLog, a.Name()) return nil } -func fileTestStoreAsset(a Asset) []*File { - return []*File{{Filename: a.Name()}} +func fileTestStoreAsset(a asset.Asset) []*asset.File { + return []*asset.File{{Filename: a.Name()}} } -func loadTestStoreAsset(a Asset) (bool, error) { +func loadTestStoreAsset(a asset.Asset) (bool, error) { return onDiskAssets[reflect.TypeOf(a)], nil } @@ -47,19 +49,19 @@ func (a *testStoreAssetA) Name() string { return "a" } -func (a *testStoreAssetA) Dependencies() []Asset { +func (a *testStoreAssetA) Dependencies() []asset.Asset { return dependenciesTestStoreAsset(a) } -func (a *testStoreAssetA) Generate(Parents) error { +func (a *testStoreAssetA) Generate(asset.Parents) error { return generateTestStoreAsset(a) } -func (a *testStoreAssetA) Files() []*File { +func (a *testStoreAssetA) Files() []*asset.File { return fileTestStoreAsset(a) } -func (a *testStoreAssetA) Load(FileFetcher) (bool, error) { +func (a *testStoreAssetA) Load(asset.FileFetcher) (bool, error) { return loadTestStoreAsset(a) } @@ -69,19 +71,19 @@ func (a *testStoreAssetB) Name() string { return "b" } -func (a *testStoreAssetB) Dependencies() []Asset { +func (a *testStoreAssetB) Dependencies() []asset.Asset { return dependenciesTestStoreAsset(a) } -func (a *testStoreAssetB) Generate(Parents) error { +func (a *testStoreAssetB) Generate(asset.Parents) error { return generateTestStoreAsset(a) } -func (a *testStoreAssetB) Files() []*File { +func (a *testStoreAssetB) Files() []*asset.File { return fileTestStoreAsset(a) } -func (a *testStoreAssetB) Load(FileFetcher) (bool, error) { +func (a *testStoreAssetB) Load(asset.FileFetcher) (bool, error) { return loadTestStoreAsset(a) } @@ -91,19 +93,19 @@ func (a *testStoreAssetC) Name() string { return "c" } -func (a *testStoreAssetC) Dependencies() []Asset { +func (a *testStoreAssetC) Dependencies() []asset.Asset { return dependenciesTestStoreAsset(a) } -func (a *testStoreAssetC) Generate(Parents) error { +func (a *testStoreAssetC) Generate(asset.Parents) error { return generateTestStoreAsset(a) } -func (a *testStoreAssetC) Files() []*File { +func (a *testStoreAssetC) Files() []*asset.File { return fileTestStoreAsset(a) } -func (a *testStoreAssetC) Load(FileFetcher) (bool, error) { +func (a *testStoreAssetC) Load(asset.FileFetcher) (bool, error) { return loadTestStoreAsset(a) } @@ -113,23 +115,23 @@ func (a *testStoreAssetD) Name() string { return "d" } -func (a *testStoreAssetD) Dependencies() []Asset { +func (a *testStoreAssetD) Dependencies() []asset.Asset { return dependenciesTestStoreAsset(a) } -func (a *testStoreAssetD) Generate(Parents) error { +func (a *testStoreAssetD) Generate(asset.Parents) error { return generateTestStoreAsset(a) } -func (a *testStoreAssetD) Files() []*File { +func (a *testStoreAssetD) Files() []*asset.File { return fileTestStoreAsset(a) } -func (a *testStoreAssetD) Load(FileFetcher) (bool, error) { +func (a *testStoreAssetD) Load(asset.FileFetcher) (bool, error) { return loadTestStoreAsset(a) } -func newTestStoreAsset(name string) Asset { +func newTestStoreAsset(name string) asset.Asset { switch name { case "a": return &testStoreAssetA{} @@ -262,16 +264,16 @@ func TestStoreFetch(t *testing.T) { t.Fatalf("failed to create temporary directory: %v", err) } defer os.RemoveAll(dir) - store := &StoreImpl{ + store := &storeImpl{ directory: dir, assets: map[reflect.Type]*assetState{}, } - assets := make(map[string]Asset, len(tc.assets)) + assets := make(map[string]asset.Asset, len(tc.assets)) for name := range tc.assets { assets[name] = newTestStoreAsset(name) } for name, deps := range tc.assets { - dependenciesOfAsset := make([]Asset, len(deps)) + dependenciesOfAsset := make([]asset.Asset, len(deps)) for i, d := range deps { dependenciesOfAsset[i] = assets[d] } @@ -361,15 +363,15 @@ func TestStoreFetchOnDiskAssets(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { clearAssetBehaviors() - store := &StoreImpl{ + store := &storeImpl{ assets: map[reflect.Type]*assetState{}, } - assets := make(map[string]Asset, len(tc.assets)) + assets := make(map[string]asset.Asset, len(tc.assets)) for name := range tc.assets { assets[name] = newTestStoreAsset(name) } for name, deps := range tc.assets { - dependenciesOfAsset := make([]Asset, len(deps)) + dependenciesOfAsset := make([]asset.Asset, len(deps)) for i, d := range deps { dependenciesOfAsset[i] = assets[d] } From 75ab106010295abdd0e79e1f39a010d03b1b8e3e Mon Sep 17 00:00:00 2001 From: staebler Date: Wed, 12 Dec 2018 15:24:00 -0500 Subject: [PATCH 5/7] assets: add tests for validating asset fetching of targets This adds tests around creating the targeted assets. The tests validate the following. 1) An asset that is generated and then fetched again from a new store has equivalent state. 2) A targeted asset restored from the state file and from on-disk has equivalent state. To keep from duplicating the code defining the assets including in the targets, the definitions have been moved to pkg/asset/targets/targets.go. Fixes https://jira.coreos.com/browse/CORS-940 --- cmd/openshift-install/create.go | 19 ++--- pkg/asset/machines/master.go | 4 + pkg/asset/machines/worker.go | 4 + pkg/asset/store/assetcreate_test.go | 113 ++++++++++++++++++++++++++++ pkg/asset/targets/targets.go | 67 +++++++++++++++++ pkg/asset/templates/doc.go | 2 + pkg/asset/templates/templates.go | 33 -------- 7 files changed, 196 insertions(+), 46 deletions(-) create mode 100644 pkg/asset/store/assetcreate_test.go create mode 100644 pkg/asset/targets/targets.go create mode 100644 pkg/asset/templates/doc.go delete mode 100644 pkg/asset/templates/templates.go diff --git a/cmd/openshift-install/create.go b/cmd/openshift-install/create.go index c5349128525..07d8e89f0a6 100644 --- a/cmd/openshift-install/create.go +++ b/cmd/openshift-install/create.go @@ -26,15 +26,8 @@ import ( configclient "github.com/openshift/client-go/config/clientset/versioned" routeclient "github.com/openshift/client-go/route/clientset/versioned" "github.com/openshift/installer/pkg/asset" - "github.com/openshift/installer/pkg/asset/cluster" - "github.com/openshift/installer/pkg/asset/ignition/bootstrap" - "github.com/openshift/installer/pkg/asset/ignition/machine" - "github.com/openshift/installer/pkg/asset/installconfig" - "github.com/openshift/installer/pkg/asset/kubeconfig" - "github.com/openshift/installer/pkg/asset/manifests" assetstore "github.com/openshift/installer/pkg/asset/store" - "github.com/openshift/installer/pkg/asset/templates" - "github.com/openshift/installer/pkg/asset/tls" + targetassets "github.com/openshift/installer/pkg/asset/targets" destroybootstrap "github.com/openshift/installer/pkg/destroy/bootstrap" cov1helpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers" ) @@ -56,7 +49,7 @@ var ( // FIXME: add longer descriptions for our commands with examples for better UX. // Long: "", }, - assets: []asset.WritableAsset{&installconfig.InstallConfig{}}, + assets: targetassets.InstallConfig, } manifestsTarget = target{ @@ -67,7 +60,7 @@ var ( // FIXME: add longer descriptions for our commands with examples for better UX. // Long: "", }, - assets: []asset.WritableAsset{&manifests.Manifests{}, &manifests.Openshift{}}, + assets: targetassets.Manifests, } manifestTemplatesTarget = target{ @@ -77,7 +70,7 @@ var ( Short: "Generates the unrendered Kubernetes manifest templates", Long: "", }, - assets: templates.Templates, + assets: targetassets.ManifestTemplates, } ignitionConfigsTarget = target{ @@ -88,7 +81,7 @@ var ( // FIXME: add longer descriptions for our commands with examples for better UX. // Long: "", }, - assets: []asset.WritableAsset{&bootstrap.Bootstrap{}, &machine.Master{}, &machine.Worker{}, &kubeconfig.Admin{}, &cluster.Metadata{}}, + assets: targetassets.IgnitionConfigs, } clusterTarget = target{ @@ -129,7 +122,7 @@ var ( } }, }, - assets: []asset.WritableAsset{&cluster.TerraformVariables{}, &kubeconfig.Admin{}, &tls.JournalCertKey{}, &cluster.Metadata{}, &cluster.Cluster{}}, + assets: targetassets.Cluster, } targets = []target{installConfigTarget, manifestTemplatesTarget, manifestsTarget, ignitionConfigsTarget, clusterTarget} diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index 08d3f376d34..609f53e67ef 100644 --- a/pkg/asset/machines/master.go +++ b/pkg/asset/machines/master.go @@ -111,6 +111,10 @@ func (m *Master) Generate(dependencies asset.Parents) error { } m.MachinesRaw = raw case nonetypes.Name: + // This is needed to ensure that roundtrip generate-load tests pass when + // comparing this value. Otherwise, generate will use a nil value while + // load will use an empty byte slice. + m.MachinesRaw = []byte{} case openstacktypes.Name: mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName) mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform) diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index d73ec0ba3be..fc0c612d42b 100644 --- a/pkg/asset/machines/worker.go +++ b/pkg/asset/machines/worker.go @@ -131,6 +131,10 @@ func (w *Worker) Generate(dependencies asset.Parents) error { } w.MachineSetRaw = raw case nonetypes.Name: + // This is needed to ensure that roundtrip generate-load tests pass when + // comparing this value. Otherwise, generate will use a nil value while + // load will use an empty byte slice. + w.MachineSetRaw = []byte{} case openstacktypes.Name: mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName) mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform) diff --git a/pkg/asset/store/assetcreate_test.go b/pkg/asset/store/assetcreate_test.go new file mode 100644 index 00000000000..795cc7e6f15 --- /dev/null +++ b/pkg/asset/store/assetcreate_test.go @@ -0,0 +1,113 @@ +package store + +import ( + "io/ioutil" + "os" + "path/filepath" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/targets" +) + +const userProvidedAssets = `{ + "*installconfig.baseDomain": { + "BaseDomain": "test-domain" + }, + "*installconfig.clusterID": { + "ClusterID": "test-cluster-id" + }, + "*installconfig.clusterName": { + "ClusterName": "test-cluster" + }, + "*installconfig.platform": { + "none": {} + }, + "*installconfig.pullSecret": { + "PullSecret": "{\"auths\": {\"example.com\": {\"auth\": \"test-auth\"}}}\n" + }, + "*installconfig.sshPublicKey": {} +}` + +func TestCreatedAssetsAreNotDirty(t *testing.T) { + cases := []struct { + name string + targets []asset.WritableAsset + }{ + { + name: "install config", + targets: targets.InstallConfig, + }, + { + name: "manifest templates", + targets: targets.ManifestTemplates, + }, + { + name: "manifests", + targets: targets.Manifests, + }, + { + name: "ignition configs", + targets: targets.IgnitionConfigs, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tempDir, err := ioutil.TempDir("", "TestCreatedAssetsAreNotDirty") + if err != nil { + t.Fatalf("could not create the temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + if err := ioutil.WriteFile(filepath.Join(tempDir, stateFileName), []byte(userProvidedAssets), 0666); err != nil { + t.Fatalf("could not write the state file: %v", err) + } + + assetStore, err := newStore(tempDir) + if err != nil { + t.Fatalf("failed to create asset store: %v", err) + } + + for _, a := range tc.targets { + if err := assetStore.Fetch(a); err != nil { + t.Fatalf("failed to fetch %q: %v", a.Name(), err) + } + + if err := asset.PersistToFile(a, tempDir); err != nil { + t.Fatalf("failed to write asset %q to disk: %v", a.Name(), err) + } + } + + newAssetStore, err := newStore(tempDir) + if err != nil { + t.Fatalf("failed to create new asset store: %v", err) + } + + for _, a := range tc.targets { + newAsset := reflect.New(reflect.TypeOf(a).Elem()).Interface().(asset.WritableAsset) + if err := newAssetStore.Fetch(newAsset); err != nil { + t.Fatalf("failed to fetch %q in new store: %v", a.Name(), err) + } + assetState := newAssetStore.assets[reflect.TypeOf(a)] + // Make an exception for metadata. It's files are read-only. + if a.Name() != "Metadata" { + assert.Truef(t, assetState.presentOnDisk, "asset %q was not found on disk", a.Name()) + } + } + + assert.Equal(t, len(assetStore.assets), len(newAssetStore.assets), "new asset store does not have the same number of assets as original") + + for _, a := range newAssetStore.assets { + originalAssetState, ok := assetStore.assets[reflect.TypeOf(a.asset)] + if !ok { + t.Fatalf("asset %q not found in original store", a.asset.Name()) + } + assert.Equalf(t, originalAssetState.asset, a.asset, "fetched and generated asset %q are not equal", a.asset.Name()) + assert.Equalf(t, stateFileSource, a.source, "asset %q was not fetched from the state file", a.asset.Name()) + } + }) + } +} diff --git a/pkg/asset/targets/targets.go b/pkg/asset/targets/targets.go new file mode 100644 index 00000000000..029ae6fe05e --- /dev/null +++ b/pkg/asset/targets/targets.go @@ -0,0 +1,67 @@ +package targets + +import ( + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/cluster" + "github.com/openshift/installer/pkg/asset/ignition/bootstrap" + "github.com/openshift/installer/pkg/asset/ignition/machine" + "github.com/openshift/installer/pkg/asset/installconfig" + "github.com/openshift/installer/pkg/asset/kubeconfig" + "github.com/openshift/installer/pkg/asset/manifests" + "github.com/openshift/installer/pkg/asset/templates/content/bootkube" + "github.com/openshift/installer/pkg/asset/templates/content/openshift" + "github.com/openshift/installer/pkg/asset/tls" +) + +var ( + // InstallConfig are the install-config targeted assets. + InstallConfig = []asset.WritableAsset{ + &installconfig.InstallConfig{}, + } + + // Manifests are the manifests targeted assets. + Manifests = []asset.WritableAsset{ + &manifests.Manifests{}, + &manifests.Openshift{}, + } + + // ManifestTemplates are the manifest-templates targeted assets. + ManifestTemplates = []asset.WritableAsset{ + &bootkube.KubeCloudConfig{}, + &bootkube.MachineConfigServerTLSSecret{}, + &bootkube.OpenshiftServiceCertSignerSecret{}, + &bootkube.Pull{}, + &bootkube.CVOOverrides{}, + &bootkube.HostEtcdServiceEndpointsKubeSystem{}, + &bootkube.KubeSystemConfigmapEtcdServingCA{}, + &bootkube.KubeSystemConfigmapRootCA{}, + &bootkube.KubeSystemSecretEtcdClient{}, + &bootkube.OpenshiftMachineConfigOperator{}, + &bootkube.OpenshiftClusterAPINamespace{}, + &bootkube.OpenshiftServiceCertSignerNamespace{}, + &bootkube.EtcdServiceKubeSystem{}, + &bootkube.HostEtcdServiceKubeSystem{}, + &openshift.BindingDiscovery{}, + &openshift.CloudCredsSecret{}, + &openshift.KubeadminPasswordSecret{}, + &openshift.RoleCloudCredsSecretReader{}, + } + + // IgnitionConfigs are the ignition-configs targeted assets. + IgnitionConfigs = []asset.WritableAsset{ + &kubeconfig.Admin{}, + &machine.Master{}, + &machine.Worker{}, + &bootstrap.Bootstrap{}, + &cluster.Metadata{}, + } + + // Cluster are the cluster targeted assets. + Cluster = []asset.WritableAsset{ + &cluster.TerraformVariables{}, + &kubeconfig.Admin{}, + &tls.JournalCertKey{}, + &cluster.Cluster{}, + &cluster.Metadata{}, + } +) diff --git a/pkg/asset/templates/doc.go b/pkg/asset/templates/doc.go new file mode 100644 index 00000000000..2b53be5baf1 --- /dev/null +++ b/pkg/asset/templates/doc.go @@ -0,0 +1,2 @@ +// Package templates deals with creating template assets that will be used by other assets +package templates diff --git a/pkg/asset/templates/templates.go b/pkg/asset/templates/templates.go deleted file mode 100644 index 7821855b8b5..00000000000 --- a/pkg/asset/templates/templates.go +++ /dev/null @@ -1,33 +0,0 @@ -// Package templates deals with creating template assets that will be used by other assets -package templates - -import ( - "github.com/openshift/installer/pkg/asset" - "github.com/openshift/installer/pkg/asset/templates/content/bootkube" - "github.com/openshift/installer/pkg/asset/templates/content/openshift" -) - -// Templates are the targeted assets for generating the dependent unrendered -// template files. -var Templates = []asset.WritableAsset{ - &bootkube.KubeCloudConfig{}, - &bootkube.MachineConfigServerTLSSecret{}, - &bootkube.OpenshiftServiceCertSignerSecret{}, - &bootkube.Pull{}, - &bootkube.CVOOverrides{}, - &bootkube.HostEtcdServiceEndpointsKubeSystem{}, - &bootkube.KubeSystemConfigmapEtcdServingCA{}, - &bootkube.KubeSystemConfigmapRootCA{}, - &bootkube.KubeSystemSecretEtcdClient{}, - &bootkube.OpenshiftMachineConfigOperator{}, - &bootkube.OpenshiftClusterAPINamespace{}, - &bootkube.OpenshiftServiceCertSignerNamespace{}, - &bootkube.EtcdServiceKubeSystem{}, - &bootkube.HostEtcdServiceKubeSystem{}, - &openshift.BindingDiscovery{}, - &openshift.CloudCredsSecret{}, - &openshift.KubeadminPasswordSecret{}, - &openshift.RoleCloudCredsSecretReader{}, - &openshift.InfrastructureCRD{}, - &openshift.NetworkCRDs{}, -} From ef1bae2120b3c11cea88db338a6ee8f6b86b8b97 Mon Sep 17 00:00:00 2001 From: staebler Date: Wed, 12 Dec 2018 16:19:49 -0500 Subject: [PATCH 6/7] assets/manifests: sort files list in manifests assets The files lists in the manifests assets need to be sorted to ensure that the assets are equivalent when read from the state file and from on disk. Changes for https://jira.coreos.com/browse/CORS-940 --- pkg/asset/asset.go | 6 ++++++ pkg/asset/manifests/openshift.go | 3 +++ pkg/asset/manifests/operators.go | 4 ++++ pkg/asset/store/filefetcher.go | 2 -- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/asset/asset.go b/pkg/asset/asset.go index 6fb4e05b8fe..4c248e79860 100644 --- a/pkg/asset/asset.go +++ b/pkg/asset/asset.go @@ -5,6 +5,7 @@ import ( "io/ioutil" "os" "path/filepath" + "sort" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -95,3 +96,8 @@ func isDirEmpty(name string) (bool, error) { } return false, err // Either not empty or error, suits both cases } + +// SortFiles sorts the specified files by file name. +func SortFiles(files []*File) { + sort.Slice(files, func(i, j int) bool { return files[i].Filename < files[j].Filename }) +} diff --git a/pkg/asset/manifests/openshift.go b/pkg/asset/manifests/openshift.go index 2b36acce790..84c70ffd1a1 100644 --- a/pkg/asset/manifests/openshift.go +++ b/pkg/asset/manifests/openshift.go @@ -146,6 +146,8 @@ func (o *Openshift) Generate(dependencies asset.Parents) error { }) } + asset.SortFiles(o.FileList) + return nil } @@ -161,5 +163,6 @@ func (o *Openshift) Load(f asset.FileFetcher) (bool, error) { return false, err } o.FileList = fileList + asset.SortFiles(o.FileList) return len(fileList) > 0, nil } diff --git a/pkg/asset/manifests/operators.go b/pkg/asset/manifests/operators.go index 84bca0c3a0e..984f4563944 100644 --- a/pkg/asset/manifests/operators.go +++ b/pkg/asset/manifests/operators.go @@ -116,6 +116,8 @@ func (m *Manifests) Generate(dependencies asset.Parents) error { m.FileList = append(m.FileList, network.Files()...) m.FileList = append(m.FileList, infra.Files()...) + asset.SortFiles(m.FileList) + return nil } @@ -264,6 +266,8 @@ func (m *Manifests) Load(f asset.FileFetcher) (bool, error) { m.FileList, m.KubeSysConfig = fileList, kubeSysConfig + asset.SortFiles(m.FileList) + return true, nil } diff --git a/pkg/asset/store/filefetcher.go b/pkg/asset/store/filefetcher.go index 03703ab1039..6c4344f9bf8 100644 --- a/pkg/asset/store/filefetcher.go +++ b/pkg/asset/store/filefetcher.go @@ -3,7 +3,6 @@ package store import ( "io/ioutil" "path/filepath" - "sort" "github.com/openshift/installer/pkg/asset" ) @@ -46,6 +45,5 @@ func (f *fileFetcher) FetchByPattern(pattern string) (files []*asset.File, err e }) } - sort.Slice(files, func(i, j int) bool { return files[i].Filename < files[j].Filename }) return files, nil } From 17a39659bed6b84faefc6599de9a3f1665249ded Mon Sep 17 00:00:00 2001 From: staebler Date: Wed, 30 Jan 2019 11:13:34 -0500 Subject: [PATCH 7/7] docs: rebuild dependency graph The Common Templates asset was removed so the dependency graph needed to be updated. Generated with: $ openshift-install graph | dot -Tsvg >docs/design/resource_dep.svg using: $ dot -V dot - graphviz version 2.40.1 (20161225.0304) --- docs/design/resource_dep.svg | 1306 +++++++++++++++++++--------------- 1 file changed, 728 insertions(+), 578 deletions(-) diff --git a/docs/design/resource_dep.svg b/docs/design/resource_dep.svg index 7a4f70526d7..c0b23f360a2 100644 --- a/docs/design/resource_dep.svg +++ b/docs/design/resource_dep.svg @@ -4,1156 +4,1306 @@ - + G - + installconfig.InstallConfig - -installconfig.InstallConfig + +installconfig.InstallConfig Target Install Config - -Target Install Config + +Target Install Config installconfig.InstallConfig->Target Install Config - - + + manifests.Manifests - -manifests.Manifests + +manifests.Manifests - + installconfig.InstallConfig->manifests.Manifests - - + + - + manifests.Ingress - -manifests.Ingress + +manifests.Ingress - + installconfig.InstallConfig->manifests.Ingress - - + + - + manifests.DNS - -manifests.DNS + +manifests.DNS - + installconfig.InstallConfig->manifests.DNS - - + + + + + +manifests.Infrastructure + +manifests.Infrastructure + + + +installconfig.InstallConfig->manifests.Infrastructure + + - + manifests.Networking - -manifests.Networking + +manifests.Networking - + installconfig.InstallConfig->manifests.Networking - - + + - + tls.IngressCertKey - -tls.IngressCertKey + +tls.IngressCertKey - + installconfig.InstallConfig->tls.IngressCertKey - - + + - + tls.MCSCertKey - -tls.MCSCertKey + +tls.MCSCertKey - + installconfig.InstallConfig->tls.MCSCertKey - - + + - + manifests.Openshift - -manifests.Openshift + +manifests.Openshift - + installconfig.InstallConfig->manifests.Openshift - - + + - + manifests.ClusterK8sIO - -manifests.ClusterK8sIO + +manifests.ClusterK8sIO - + installconfig.InstallConfig->manifests.ClusterK8sIO - - + + - + machines.Worker - -machines.Worker + +machines.Worker - + installconfig.InstallConfig->machines.Worker - - + + + + + +installconfig.PlatformCredsCheck + +installconfig.PlatformCredsCheck + + + +installconfig.InstallConfig->installconfig.PlatformCredsCheck + + - + rhcos.Image - -rhcos.Image + +rhcos.Image - + installconfig.InstallConfig->rhcos.Image - - + + - + machine.Worker - -machine.Worker + +machine.Worker - + installconfig.InstallConfig->machine.Worker - - + + - + machines.Master - -machines.Master + +machines.Master - + installconfig.InstallConfig->machines.Master - - + + - + machine.Master - -machine.Master + +machine.Master - + installconfig.InstallConfig->machine.Master - - + + - + bootstrap.Bootstrap - -bootstrap.Bootstrap + +bootstrap.Bootstrap - + installconfig.InstallConfig->bootstrap.Bootstrap - - + + - + tls.APIServerCertKey - -tls.APIServerCertKey + +tls.APIServerCertKey - + installconfig.InstallConfig->tls.APIServerCertKey - - + + - + kubeconfig.Admin - -kubeconfig.Admin + +kubeconfig.Admin - + installconfig.InstallConfig->kubeconfig.Admin - - + + - + kubeconfig.Kubelet - -kubeconfig.Kubelet + +kubeconfig.Kubelet - + installconfig.InstallConfig->kubeconfig.Kubelet - - + + + + + +cluster.Metadata + +cluster.Metadata + + + +installconfig.InstallConfig->cluster.Metadata + + - + cluster.TerraformVariables - -cluster.TerraformVariables + +cluster.TerraformVariables - + installconfig.InstallConfig->cluster.TerraformVariables - - + + - + cluster.Cluster - -cluster.Cluster + +cluster.Cluster - + installconfig.InstallConfig->cluster.Cluster - - - - - -installconfig.clusterID - -installconfig.clusterID - - - -installconfig.clusterID->installconfig.InstallConfig - - + + - + installconfig.sshPublicKey - -installconfig.sshPublicKey + +installconfig.sshPublicKey - + installconfig.sshPublicKey->installconfig.InstallConfig - - + + - + installconfig.baseDomain - -installconfig.baseDomain + +installconfig.baseDomain - + installconfig.baseDomain->installconfig.InstallConfig - - + + - + installconfig.platform - -installconfig.platform + +installconfig.platform - + installconfig.platform->installconfig.InstallConfig - - + + - + installconfig.platform->installconfig.baseDomain - - + + - + installconfig.clusterName - -installconfig.clusterName + +installconfig.clusterName - + installconfig.clusterName->installconfig.InstallConfig - - + + - + installconfig.pullSecret - -installconfig.pullSecret + +installconfig.pullSecret - + installconfig.pullSecret->installconfig.InstallConfig - - + + - + templates.Templates - -templates.Templates + +templates.Templates - + Target Manifest templates - -Target Manifest templates + +Target Manifest templates - + templates.Templates->Target Manifest templates - - + + - + bootkube.KubeCloudConfig - -bootkube.KubeCloudConfig + +bootkube.KubeCloudConfig - + bootkube.KubeCloudConfig->templates.Templates - - + + - + bootkube.KubeCloudConfig->manifests.Manifests - - + + - + bootkube.MachineConfigServerTLSSecret - -bootkube.MachineConfigServerTLSSecret + +bootkube.MachineConfigServerTLSSecret - + bootkube.MachineConfigServerTLSSecret->templates.Templates - - + + - + bootkube.MachineConfigServerTLSSecret->manifests.Manifests - - + + - + bootkube.OpenshiftServiceCertSignerSecret - -bootkube.OpenshiftServiceCertSignerSecret + +bootkube.OpenshiftServiceCertSignerSecret - + bootkube.OpenshiftServiceCertSignerSecret->templates.Templates - - + + - + bootkube.OpenshiftServiceCertSignerSecret->manifests.Manifests - - + + - + bootkube.Pull - -bootkube.Pull + +bootkube.Pull - + bootkube.Pull->templates.Templates - - + + - + bootkube.Pull->manifests.Manifests - - + + - + bootkube.CVOOverrides - -bootkube.CVOOverrides + +bootkube.CVOOverrides - + bootkube.CVOOverrides->templates.Templates - - + + - + bootkube.CVOOverrides->manifests.Manifests - - + + - + bootkube.HostEtcdServiceEndpointsKubeSystem - -bootkube.HostEtcdServiceEndpointsKubeSystem + +bootkube.HostEtcdServiceEndpointsKubeSystem - + bootkube.HostEtcdServiceEndpointsKubeSystem->templates.Templates - - + + - + bootkube.HostEtcdServiceEndpointsKubeSystem->manifests.Manifests - - + + - + bootkube.KubeSystemConfigmapEtcdServingCA - -bootkube.KubeSystemConfigmapEtcdServingCA + +bootkube.KubeSystemConfigmapEtcdServingCA - + bootkube.KubeSystemConfigmapEtcdServingCA->templates.Templates - - + + - + bootkube.KubeSystemConfigmapEtcdServingCA->manifests.Manifests - - + + - + bootkube.KubeSystemConfigmapRootCA - -bootkube.KubeSystemConfigmapRootCA + +bootkube.KubeSystemConfigmapRootCA - + bootkube.KubeSystemConfigmapRootCA->templates.Templates - - + + - + bootkube.KubeSystemConfigmapRootCA->manifests.Manifests - - + + - + bootkube.KubeSystemSecretEtcdClient - -bootkube.KubeSystemSecretEtcdClient + +bootkube.KubeSystemSecretEtcdClient - + bootkube.KubeSystemSecretEtcdClient->templates.Templates - - + + - + bootkube.KubeSystemSecretEtcdClient->manifests.Manifests - - + + - + bootkube.OpenshiftMachineConfigOperator - -bootkube.OpenshiftMachineConfigOperator + +bootkube.OpenshiftMachineConfigOperator - + bootkube.OpenshiftMachineConfigOperator->templates.Templates - - + + - + bootkube.OpenshiftMachineConfigOperator->manifests.Manifests - - + + - + bootkube.OpenshiftClusterAPINamespace - -bootkube.OpenshiftClusterAPINamespace + +bootkube.OpenshiftClusterAPINamespace - + bootkube.OpenshiftClusterAPINamespace->templates.Templates - - + + - + bootkube.OpenshiftClusterAPINamespace->manifests.Manifests - - + + - + bootkube.OpenshiftServiceCertSignerNamespace - -bootkube.OpenshiftServiceCertSignerNamespace + +bootkube.OpenshiftServiceCertSignerNamespace - + bootkube.OpenshiftServiceCertSignerNamespace->templates.Templates - - + + - + bootkube.OpenshiftServiceCertSignerNamespace->manifests.Manifests - - + + - + bootkube.EtcdServiceKubeSystem - -bootkube.EtcdServiceKubeSystem + +bootkube.EtcdServiceKubeSystem - + bootkube.EtcdServiceKubeSystem->templates.Templates - - + + - + bootkube.EtcdServiceKubeSystem->manifests.Manifests - - + + - + bootkube.HostEtcdServiceKubeSystem - -bootkube.HostEtcdServiceKubeSystem + +bootkube.HostEtcdServiceKubeSystem - + bootkube.HostEtcdServiceKubeSystem->templates.Templates - - + + - + bootkube.HostEtcdServiceKubeSystem->manifests.Manifests - - + + - + openshift.BindingDiscovery - -openshift.BindingDiscovery + +openshift.BindingDiscovery - + openshift.BindingDiscovery->templates.Templates - - + + - + openshift.BindingDiscovery->manifests.Openshift - - + + - + openshift.CloudCredsSecret - -openshift.CloudCredsSecret + +openshift.CloudCredsSecret - + openshift.CloudCredsSecret->templates.Templates - - + + - + openshift.CloudCredsSecret->manifests.Openshift - - + + - + openshift.KubeadminPasswordSecret - -openshift.KubeadminPasswordSecret + +openshift.KubeadminPasswordSecret - + openshift.KubeadminPasswordSecret->templates.Templates - - + + - + openshift.KubeadminPasswordSecret->manifests.Openshift - - + + - + openshift.RoleCloudCredsSecretReader - -openshift.RoleCloudCredsSecretReader + +openshift.RoleCloudCredsSecretReader - + openshift.RoleCloudCredsSecretReader->templates.Templates - - + + - + openshift.RoleCloudCredsSecretReader->manifests.Openshift - - + + + + + +openshift.InfrastructureCRD + +openshift.InfrastructureCRD + + + +openshift.InfrastructureCRD->templates.Templates + + + + + +openshift.InfrastructureCRD->manifests.Infrastructure + + Target Manifests - -Target Manifests + +Target Manifests manifests.Manifests->Target Manifests - - + + - + manifests.Manifests->bootstrap.Bootstrap - - + + + + + +installconfig.ClusterID + +installconfig.ClusterID + + + +installconfig.ClusterID->manifests.Manifests + + + + + +installconfig.ClusterID->machines.Worker + + + + + +installconfig.ClusterID->machines.Master + + + + + +installconfig.ClusterID->cluster.Metadata + + + + + +installconfig.ClusterID->cluster.TerraformVariables + + + + + +installconfig.ClusterID->cluster.Cluster + + - + manifests.Ingress->manifests.Manifests - - + + - + manifests.DNS->manifests.Manifests - - + + + + + +manifests.Infrastructure->manifests.Manifests + + - + manifests.Networking->manifests.Manifests - - + + - + manifests.Networking->manifests.ClusterK8sIO - - + + - + tls.RootCA - -tls.RootCA + +tls.RootCA - + tls.RootCA->manifests.Manifests - - + + - + tls.EtcdCA - -tls.EtcdCA + +tls.EtcdCA - + tls.RootCA->tls.EtcdCA - - + + - + tls.KubeCA - -tls.KubeCA + +tls.KubeCA - + tls.RootCA->tls.KubeCA - - + + - + tls.ServiceServingCA - -tls.ServiceServingCA + +tls.ServiceServingCA - + tls.RootCA->tls.ServiceServingCA - - + + - + tls.RootCA->tls.MCSCertKey - - + + - + tls.RootCA->machine.Worker - - + + - + tls.RootCA->machine.Master - - + + - + tls.RootCA->bootstrap.Bootstrap - - + + - + tls.AggregatorCA - -tls.AggregatorCA + +tls.AggregatorCA - + tls.RootCA->tls.AggregatorCA - - + + + + + +tls.JournalCertKey + +tls.JournalCertKey + + + +tls.RootCA->tls.JournalCertKey + + - + tls.RootCA->kubeconfig.Admin - - + + - + tls.RootCA->kubeconfig.Kubelet - - + + - + tls.EtcdCA->manifests.Manifests - - + + - + tls.EtcdClientCertKey - -tls.EtcdClientCertKey + +tls.EtcdClientCertKey - + tls.EtcdCA->tls.EtcdClientCertKey - - + + - + tls.EtcdCA->bootstrap.Bootstrap - - + + - + tls.IngressCertKey->manifests.Manifests - - + + - + tls.KubeCA->manifests.Manifests - - + + - + tls.KubeCA->tls.IngressCertKey - - + + - + tls.KubeletCertKey - -tls.KubeletCertKey + +tls.KubeletCertKey - + tls.KubeCA->tls.KubeletCertKey - - + + - + tls.KubeCA->bootstrap.Bootstrap - - + + - + tls.KubeCA->tls.APIServerCertKey - - + + - + tls.AdminCertKey - -tls.AdminCertKey + +tls.AdminCertKey - + tls.KubeCA->tls.AdminCertKey - - + + - + tls.ServiceServingCA->manifests.Manifests - - + + - + tls.ServiceServingCA->bootstrap.Bootstrap - - + + - + tls.EtcdClientCertKey->manifests.Manifests - - + + - + tls.EtcdClientCertKey->bootstrap.Bootstrap - - + + - + tls.MCSCertKey->manifests.Manifests - - + + - + tls.MCSCertKey->bootstrap.Bootstrap - - + + - + tls.KubeletCertKey->manifests.Manifests - - + + - + tls.KubeletCertKey->bootstrap.Bootstrap - - + + - + tls.KubeletCertKey->kubeconfig.Kubelet - - + + - + manifests.Openshift->Target Manifests - - + + - + manifests.Openshift->bootstrap.Bootstrap - - + + - + manifests.ClusterK8sIO->manifests.Openshift - - + + - + machines.Worker->manifests.Openshift - - + + + + + +installconfig.PlatformCredsCheck->machines.Worker + + + + + +installconfig.PlatformCredsCheck->machines.Master + + + + + +installconfig.PlatformCredsCheck->cluster.Cluster + + - + rhcos.Image->machines.Worker - - + + - + rhcos.Image->machines.Master - - + + - + rhcos.Image->cluster.TerraformVariables - - + + - + machine.Worker->machines.Worker - - + + - + Target Ignition Configs - -Target Ignition Configs + +Target Ignition Configs - + machine.Worker->Target Ignition Configs - - + + - + machines.Master->manifests.Openshift - - + + - + machine.Master->machines.Master - - + + - + machine.Master->Target Ignition Configs - - + + - + machine.Master->cluster.TerraformVariables - - + + - + password.KubeadminPassword - -password.KubeadminPassword + +password.KubeadminPassword - + password.KubeadminPassword->manifests.Openshift - - + + - + password.KubeadminPassword->cluster.Cluster - - + + - + bootstrap.Bootstrap->Target Ignition Configs - - + + - + bootstrap.Bootstrap->cluster.TerraformVariables - - + + - + tls.AggregatorCA->bootstrap.Bootstrap - - + + - + tls.APIServerProxyCertKey - -tls.APIServerProxyCertKey + +tls.APIServerProxyCertKey - + tls.AggregatorCA->tls.APIServerProxyCertKey - - + + - + tls.APIServerCertKey->bootstrap.Bootstrap - - + + - + tls.APIServerProxyCertKey->bootstrap.Bootstrap - - + + - + tls.AdminCertKey->bootstrap.Bootstrap - - + + - + tls.AdminCertKey->kubeconfig.Admin - - + + - + tls.ServiceAccountKeyPair - -tls.ServiceAccountKeyPair + +tls.ServiceAccountKeyPair - + tls.ServiceAccountKeyPair->bootstrap.Bootstrap - - + + - - -kubeconfig.Admin->bootstrap.Bootstrap - - + + +tls.JournalCertKey->bootstrap.Bootstrap + + - + Target Cluster - -Target Cluster + +Target Cluster + + + +tls.JournalCertKey->Target Cluster + + + + + +kubeconfig.Admin->bootstrap.Bootstrap + + + + + +kubeconfig.Admin->Target Ignition Configs + + - + kubeconfig.Admin->Target Cluster - - + + - + kubeconfig.Kubelet->bootstrap.Bootstrap - - + + + + + +cluster.Metadata->Target Ignition Configs + + + + + +cluster.Metadata->Target Cluster + + - + cluster.TerraformVariables->Target Cluster - - + + - + cluster.TerraformVariables->cluster.Cluster - - + + - + cluster.Cluster->Target Cluster - - + +