diff --git a/pkg/common/utils.go b/pkg/common/utils.go index c93a96319..8d6ed0915 100644 --- a/pkg/common/utils.go +++ b/pkg/common/utils.go @@ -172,7 +172,10 @@ func GetHCP(ctx context.Context, nsList []string, c crclient.Client, log logrus. return &hcpList.Items[0], nil } - return nil, fmt.Errorf("no HostedControlPlane found") + return nil, apierrors.NewNotFound( + hyperv1.Resource("hostedcontrolplanes"), + strings.Join(nsList, ","), + ) } func GetHCPNamespace(name, namespace string) string { diff --git a/pkg/common/utils_test.go b/pkg/common/utils_test.go index eacfb8871..59786b624 100644 --- a/pkg/common/utils_test.go +++ b/pkg/common/utils_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/gomega" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" @@ -323,6 +324,21 @@ func TestGetHCP(t *testing.T) { } } +func TestGetHCPReturnsNotFoundError(t *testing.T) { + g := NewWithT(t) + + scheme := runtime.NewScheme() + _ = hyperv1.AddToScheme(scheme) + + c := fake.NewClientBuilder().WithScheme(scheme).Build() + log := logrus.New() + + hcp, err := GetHCP(context.TODO(), []string{"ns1", "ns2"}, c, log) + g.Expect(hcp).To(BeNil()) + g.Expect(err).To(HaveOccurred()) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "GetHCP should return a NotFound API error when no HCP exists") +} + func TestAddLabel(t *testing.T) { tests := []struct { name string diff --git a/pkg/core/backup.go b/pkg/core/backup.go index fcf9e1bd2..95955bccc 100644 --- a/pkg/core/backup.go +++ b/pkg/core/backup.go @@ -27,10 +27,11 @@ type BackupPlugin struct { log logrus.FieldLogger ctx context.Context - client crclient.Client - config map[string]string - validator validation.BackupValidator - hcp *hyperv1.HostedControlPlane + client crclient.Client + config map[string]string + validator validation.BackupValidator + hcp *hyperv1.HostedControlPlane + hcpNotFound bool *plugtypes.BackupOptions // Etcd backup orchestration @@ -118,7 +119,9 @@ func (p *BackupPlugin) Name() string { } func (p *BackupPlugin) AppliesTo() (velero.ResourceSelector, error) { - return velero.ResourceSelector{}, nil + return velero.ResourceSelector{ + IncludedResources: plugtypes.AllPluginResources, + }, nil } // Execute allows the ItemAction to perform arbitrary logic with the item being backed up, @@ -131,15 +134,21 @@ func (p *BackupPlugin) Execute(item runtime.Unstructured, backup *velerov1.Backu return item, nil, nil } + // set to true below when GetHCP returns NotFound for this backup's namespaces + if p.hcpNotFound { + return item, nil, nil + } + if p.hcp == nil { var err error p.hcp, err = common.GetHCP(ctx, backup.Spec.IncludedNamespaces, p.client, p.log) if err != nil { if apierrors.IsNotFound(err) { - p.log.Infof("HCP not found, assuming not hypershift cluster to backup") + p.log.Infof("HCP not found in included namespaces, skipping plugin") + p.hcpNotFound = true return item, nil, nil } - return nil, nil, fmt.Errorf("error getting HCP namespace: %v", err) + return nil, nil, fmt.Errorf("error getting HCP namespace: %w", err) } } diff --git a/pkg/core/backup_test.go b/pkg/core/backup_test.go index 8f62877ab..07ff12ceb 100644 --- a/pkg/core/backup_test.go +++ b/pkg/core/backup_test.go @@ -89,6 +89,87 @@ func newTestBackup() *velerov1.Backup { } } +func TestAppliesToReturnsSpecificResources(t *testing.T) { + g := NewWithT(t) + bp := newTestBackupPlugin() + + selector, err := bp.AppliesTo() + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(selector.IncludedResources).NotTo(BeEmpty(), "AppliesTo must not return an empty resource selector") + + // Per-provider happy-path assertions: at least one representative resource per platform. + // These ensure that accidentally dropping a platform slice is caught immediately. + for _, tc := range []struct { + provider string + resources []string + }{ + {"common", plugtypes.BackupCommonResources}, + {"aws", plugtypes.BackupAWSResources}, + {"azure", plugtypes.BackupAzureResources}, + {"ibmpowervs", plugtypes.BackupIBMPowerVSResources}, + {"openstack", plugtypes.BackupOpenStackResources}, + {"kubevirt", plugtypes.BackupKubevirtResources}, + {"agent", plugtypes.BackupAgentResources}, + } { + for _, r := range tc.resources { + g.Expect(selector.IncludedResources).To( + ContainElement(r), + "provider %q resource %q must appear in AppliesTo", tc.provider, r, + ) + } + } + + // Sad-path assertion: unknown resources must NOT appear. + g.Expect(selector.IncludedResources).NotTo(ContainElement("completelyunknownresource")) +} + +func TestExecuteSkipsWhenHCPNotFound(t *testing.T) { + g := NewWithT(t) + + scheme := common.CustomScheme + + hcpCRD := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "hostedcontrolplanes.hypershift.openshift.io"}, + } + + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(hcpCRD). + Build() + + bp := &BackupPlugin{ + log: logrus.New(), + ctx: context.Background(), + client: client, + config: map[string]string{}, + validator: &mockValidator{}, + BackupOptions: &plugtypes.BackupOptions{}, + hoNamespace: "hypershift", + etcdBackupMethod: common.EtcdBackupMethodVolume, + } + + backup := &velerov1.Backup{ + ObjectMeta: metav1.ObjectMeta{Name: "test-backup", Namespace: "openshift-adp"}, + Spec: velerov1.BackupSpec{ + IncludedNamespaces: []string{"clusters-test"}, + IncludedResources: []string{"hostedcontrolplanes"}, + }, + } + + item := newUnstructuredItem("Secret", "v1", "my-secret", "clusters-test") + + // First call should gracefully skip (no HCP in namespace) + result, _, err := bp.Execute(item, backup) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result).NotTo(BeNil(), "should return item, not error") + g.Expect(bp.hcpNotFound).To(BeTrue(), "should cache hcpNotFound=true") + + // Second call should return immediately without re-querying + result2, _, err2 := bp.Execute(item, backup) + g.Expect(err2).NotTo(HaveOccurred()) + g.Expect(result2).NotTo(BeNil()) +} + func TestExecute(t *testing.T) { falseVal := false diff --git a/pkg/core/restore.go b/pkg/core/restore.go index febdbc4fa..484bef15c 100644 --- a/pkg/core/restore.go +++ b/pkg/core/restore.go @@ -3,7 +3,6 @@ package core import ( "context" "fmt" - "slices" "strings" hive "github.com/openshift/hive/apis/hive/v1" @@ -110,15 +109,7 @@ func (p *RestorePlugin) Name() string { func (p *RestorePlugin) AppliesTo() (velero.ResourceSelector, error) { return velero.ResourceSelector{ - IncludedResources: slices.Concat( - plugtypes.BackupCommonResources, - plugtypes.BackupAWSResources, - plugtypes.BackupAzureResources, - plugtypes.BackupIBMPowerVSResources, - plugtypes.BackupOpenStackResources, - plugtypes.BackupKubevirtResources, - plugtypes.BackupAgentResources, - ), + IncludedResources: plugtypes.AllPluginResources, }, nil } diff --git a/pkg/core/types/types.go b/pkg/core/types/types.go index 5cd0d1944..71061fe3c 100644 --- a/pkg/core/types/types.go +++ b/pkg/core/types/types.go @@ -1,5 +1,7 @@ package types +import "slices" + var ( BackupCommonResources = []string{ "hostedclusters", "hostedcluster", "hostedcontrolplanes", "hostedcontrolplane", "nodepools", "nodepool", @@ -15,7 +17,19 @@ var ( BackupIBMPowerVSResources = []string{"ibmpowervsmachines", "ibmpowervsmachinetemplates", "ibmpowervsclusters", "ibmpowervsclustertemplates"} BackupOpenStackResources = []string{"openstackmachines", "openstackmachinetemplates", "openstackclusters", "openstackclustertemplates"} BackupKubevirtResources = []string{"kubevirtcluster", "kubevirtmachinetemplate", "datavolume"} - BackupAgentResources = []string{"agents", "agentmachines", "agentmachinetemplates", "agentmachinepools", "agentclusters", "nmstateconfigs", "nmstateconfig", "infraenvs", "infraenv"} + BackupAgentResources = []string{"agents", "agentmachines", "agentmachinetemplates", "agentmachinepools", "agentclusters", "clusterdeployments", "clusterdeployment", "nmstateconfigs", "nmstateconfig", "infraenvs", "infraenv"} + + // AllPluginResources is the combined set of all per-platform resource lists. + // Use this single source of truth in AppliesTo() for both backup and restore plugins. + AllPluginResources = slices.Concat( + BackupCommonResources, + BackupAWSResources, + BackupAzureResources, + BackupIBMPowerVSResources, + BackupOpenStackResources, + BackupKubevirtResources, + BackupAgentResources, + ) ) type BackupOptions struct {