Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Debug Information Paths to NAB/NAR CR status #154

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

kaovilai
Copy link
Member

@kaovilai kaovilai commented Jan 28, 2025

Signed-off-by: Tiger Kaovilai [email protected]

Why the changes were made

This PR adds logs and resourceList to NAB/NAR CR status on their completion status update.

oadp-operator PR: openshift/oadp-operator#1628

Fixes #132

How to test the changes made

Create NAB or NAR and wait till completion and check if status contain populated paths of logs and resourceList .gz files.

Dev test results

TBC

make docker-build IMG=$(ghcr_notag):132
docker build --platform=$(dockerplatforms-amdarmibm) --push -t ghcr.io/kaovilai/oadp-non-admin:132 .

Copy link

openshift-ci bot commented Jan 28, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link

openshift-ci bot commented Jan 28, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mpryc

This comment was marked as outdated.

@kaovilai kaovilai force-pushed the design/132 branch 2 times, most recently from 3e67c7a to 8795ce6 Compare February 6, 2025 05:45
@kaovilai kaovilai changed the title WIP: Design: Debug Information Download URL Debug Information Download URL Feb 6, 2025
@kaovilai kaovilai force-pushed the design/132 branch 2 times, most recently from 0ffa920 to ddb85fe Compare February 6, 2025 05:52
@kaovilai kaovilai changed the title Debug Information Download URL Add Debug Information Paths to NAB/NAR CR Feb 6, 2025
@kaovilai kaovilai marked this pull request as ready for review February 6, 2025 05:53
@kaovilai kaovilai changed the title Add Debug Information Paths to NAB/NAR CR Add Debug Information Paths to NAB/NAR CR status Feb 6, 2025
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests? or skunk works mode.

@kaovilai kaovilai marked this pull request as draft February 6, 2025 06:16
Copy link
Member Author

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB controller_test.go contain simulated velero runs which means the fake envtest env need to also create BSL to simulate an actual BSL with all specs filled out.

This should be sufficient unit test IMO. 0/ 🛏️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to create a new folder?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So these could be independently imported by other projects if needed without bringing along many other NAC dependencies.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although that's not really happening

Details

go list --deps ./pkg/velero/storagelocation/paths/... 
internal/goarch
unsafe
internal/abi
internal/unsafeheader
internal/cpu
internal/bytealg
internal/byteorder
internal/chacha8rand
internal/coverage/rtcov
internal/godebugs
internal/goexperiment
internal/goos
internal/profilerecord
internal/runtime/atomic
internal/runtime/exithook
internal/stringslite
runtime/internal/math
runtime/internal/sys
runtime
internal/reflectlite
errors
internal/race
sync/atomic
sync
internal/bisect
internal/godebug
internal/asan
internal/itoa
internal/msan
internal/oserror
syscall
time
context
cmp
iter
math/bits
math
unicode/utf8
strconv
unicode
reflect
slices
internal/fmtsort
io
path
io/fs
internal/filepathlite
internal/syscall/unix
internal/poll
internal/syscall/execenv
internal/testlog
os
fmt
github.com/vmware-tanzu/velero/pkg/apis/velero/shared
bytes
strings
bufio
encoding
encoding/binary
encoding/base64
unicode/utf16
encoding/json
log/internal
log
sort
github.com/gogo/protobuf/proto
github.com/gogo/protobuf/sortkeys
math/rand
math/big
gopkg.in/inf.v0
k8s.io/apimachinery/pkg/api/resource
github.com/google/gofuzz/bytesource
regexp/syntax
regexp
github.com/google/gofuzz
k8s.io/apimachinery/third_party/forked/golang/reflect
k8s.io/apimachinery/pkg/conversion
k8s.io/apimachinery/pkg/selection
k8s.io/apimachinery/pkg/fields
k8s.io/apimachinery/pkg/util/sets
k8s.io/apimachinery/pkg/util/errors
k8s.io/apimachinery/pkg/util/validation/field
vendor/golang.org/x/net/dns/dnsmessage
vendor/golang.org/x/net/route
internal/nettrace
internal/singleflight
math/rand/v2
internal/concurrent
internal/weak
unique
net/netip
net
k8s.io/utils/internal/third_party/forked/golang/net
k8s.io/utils/net
k8s.io/apimachinery/pkg/util/validation
flag
log/slog/internal
log/slog/internal/buffer
log/slog
github.com/go-logr/logr
k8s.io/klog/v2/internal/severity
k8s.io/klog/v2/internal/buffer
k8s.io/klog/v2/internal/clock
k8s.io/klog/v2/internal/dbg
k8s.io/klog/v2/internal/serialize
k8s.io/klog/v2/internal/sloghandler
os/user
path/filepath
k8s.io/klog/v2
k8s.io/utils/strings/slices
k8s.io/apimachinery/pkg/labels
go/token
go/scanner
go/ast
go/doc/comment
internal/lazyregexp
go/doc
go/build/constraint
go/internal/typeparams
go/parser
net/url
k8s.io/apimachinery/pkg/conversion/queryparams
k8s.io/apimachinery/pkg/runtime/schema
sigs.k8s.io/json/internal/golang/encoding/json
sigs.k8s.io/json
k8s.io/apimachinery/pkg/util/json
runtime/debug
k8s.io/apimachinery/pkg/util/naming
compress/flate
hash
hash/crc32
compress/gzip
container/list
crypto
crypto/internal/alias
crypto/subtle
crypto/cipher
crypto/internal/boring/sig
crypto/internal/boring
crypto/internal/randutil
crypto/rand
crypto/aes
crypto/des
crypto/internal/edwards25519/field
crypto/internal/nistec/fiat
embed
crypto/internal/nistec
crypto/ecdh
crypto/elliptic
crypto/internal/bigmod
crypto/internal/boring/bbig
crypto/sha512
encoding/asn1
vendor/golang.org/x/crypto/cryptobyte/asn1
vendor/golang.org/x/crypto/cryptobyte
crypto/ecdsa
crypto/internal/edwards25519
crypto/ed25519
crypto/hmac
vendor/golang.org/x/crypto/internal/alias
vendor/golang.org/x/crypto/chacha20
vendor/golang.org/x/crypto/internal/poly1305
vendor/golang.org/x/crypto/chacha20poly1305
vendor/golang.org/x/crypto/hkdf
crypto/internal/hpke
vendor/golang.org/x/sys/cpu
vendor/golang.org/x/crypto/sha3
crypto/internal/mlkem768
crypto/md5
crypto/rc4
crypto/rsa
crypto/sha1
crypto/sha256
crypto/dsa
crypto/x509/internal/macos
encoding/hex
crypto/x509/pkix
encoding/pem
crypto/x509
crypto/tls
vendor/golang.org/x/text/transform
vendor/golang.org/x/text/unicode/bidi
vendor/golang.org/x/text/secure/bidirule
vendor/golang.org/x/text/unicode/norm
vendor/golang.org/x/net/idna
net/textproto
vendor/golang.org/x/net/http/httpguts
vendor/golang.org/x/net/http/httpproxy
vendor/golang.org/x/net/http2/hpack
maps
mime
mime/quotedprintable
mime/multipart
net/http/httptrace
net/http/internal
net/http/internal/ascii
net/http
k8s.io/apimachinery/pkg/util/runtime
io/ioutil
github.com/modern-go/concurrent
github.com/modern-go/reflect2
github.com/json-iterator/go
gopkg.in/yaml.v2
sigs.k8s.io/structured-merge-diff/v4/value
k8s.io/apimachinery/pkg/runtime
k8s.io/apimachinery/pkg/types
k8s.io/apimachinery/pkg/util/intstr
golang.org/x/text/transform
golang.org/x/text/unicode/bidi
golang.org/x/text/secure/bidirule
golang.org/x/text/unicode/norm
golang.org/x/net/idna
golang.org/x/net/http/httpguts
golang.org/x/net/http2/hpack
golang.org/x/net/http2
k8s.io/apimachinery/pkg/util/net
k8s.io/apimachinery/pkg/watch
k8s.io/apimachinery/pkg/apis/meta/v1
k8s.io/api/core/v1
github.com/vmware-tanzu/velero/pkg/apis/velero/v1
github.com/evanphx/json-patch/v5/internal/json
github.com/pkg/errors
github.com/evanphx/json-patch/v5
k8s.io/apimachinery/pkg/api/errors
k8s.io/apimachinery/pkg/api/meta
k8s.io/apimachinery/pkg/runtime/serializer/recognizer
k8s.io/apimachinery/pkg/util/framer
sigs.k8s.io/yaml/goyaml.v2
sigs.k8s.io/yaml
k8s.io/apimachinery/pkg/util/yaml
k8s.io/apimachinery/pkg/runtime/serializer/json
k8s.io/apimachinery/pkg/runtime/serializer/protobuf
k8s.io/apimachinery/pkg/apis/meta/v1/unstructured
k8s.io/apimachinery/pkg/runtime/serializer/versioning
k8s.io/apimachinery/pkg/runtime/serializer
github.com/davecgh/go-spew/spew
k8s.io/apimachinery/pkg/util/dump
k8s.io/apimachinery/pkg/util/mergepatch
k8s.io/apimachinery/third_party/forked/golang/json
hash/fnv
google.golang.org/protobuf/internal/detrand
google.golang.org/protobuf/internal/errors
google.golang.org/protobuf/encoding/protowire
google.golang.org/protobuf/internal/pragma
google.golang.org/protobuf/reflect/protoreflect
google.golang.org/protobuf/internal/encoding/messageset
google.golang.org/protobuf/internal/flags
google.golang.org/protobuf/internal/strs
google.golang.org/protobuf/internal/encoding/text
google.golang.org/protobuf/internal/genid
google.golang.org/protobuf/internal/order
google.golang.org/protobuf/internal/set
google.golang.org/protobuf/reflect/protoregistry
google.golang.org/protobuf/runtime/protoiface
google.golang.org/protobuf/proto
google.golang.org/protobuf/encoding/prototext
google.golang.org/protobuf/internal/editiondefaults
google.golang.org/protobuf/internal/descfmt
google.golang.org/protobuf/internal/descopts
google.golang.org/protobuf/internal/encoding/defval
google.golang.org/protobuf/internal/filedesc
google.golang.org/protobuf/internal/encoding/tag
google.golang.org/protobuf/internal/impl
google.golang.org/protobuf/internal/filetype
google.golang.org/protobuf/internal/version
google.golang.org/protobuf/runtime/protoimpl
google.golang.org/protobuf/types/descriptorpb
google.golang.org/protobuf/internal/editionssupport
google.golang.org/protobuf/types/gofeaturespb
google.golang.org/protobuf/reflect/protodesc
github.com/golang/protobuf/proto
google.golang.org/protobuf/types/known/anypb
github.com/golang/protobuf/ptypes/any
google.golang.org/protobuf/types/known/durationpb
github.com/golang/protobuf/ptypes/duration
google.golang.org/protobuf/types/known/timestamppb
github.com/golang/protobuf/ptypes/timestamp
github.com/golang/protobuf/ptypes
github.com/google/gnostic-models/extensions
gopkg.in/yaml.v3
github.com/google/gnostic-models/jsonschema
os/exec
github.com/google/gnostic-models/compiler
github.com/google/gnostic-models/openapiv2
github.com/google/gnostic-models/openapiv3
k8s.io/kube-openapi/pkg/util/proto
github.com/josharian/intern
github.com/mailru/easyjson/jlexer
github.com/mailru/easyjson/buffer
github.com/mailru/easyjson/jwriter
github.com/go-openapi/swag
github.com/go-openapi/jsonpointer
github.com/go-openapi/jsonreference/internal
github.com/go-openapi/jsonreference
encoding/base32
k8s.io/kube-openapi/pkg/internal/third_party/go-json-experiment/json
k8s.io/kube-openapi/pkg/internal
k8s.io/kube-openapi/pkg/validation/spec
k8s.io/apimachinery/pkg/util/strategicpatch
k8s.io/api/admissionregistration/v1
k8s.io/api/admissionregistration/v1alpha1
k8s.io/api/admissionregistration/v1beta1
k8s.io/api/apiserverinternal/v1alpha1
k8s.io/api/apps/v1
k8s.io/api/apps/v1beta1
k8s.io/api/apps/v1beta2
k8s.io/api/authentication/v1
k8s.io/api/authentication/v1alpha1
k8s.io/api/authentication/v1beta1
k8s.io/api/authorization/v1
k8s.io/api/authorization/v1beta1
k8s.io/api/autoscaling/v1
k8s.io/api/autoscaling/v2
k8s.io/api/autoscaling/v2beta1
k8s.io/api/autoscaling/v2beta2
k8s.io/api/batch/v1
k8s.io/api/batch/v1beta1
k8s.io/api/certificates/v1
k8s.io/api/certificates/v1alpha1
k8s.io/api/certificates/v1beta1
k8s.io/api/coordination/v1
k8s.io/api/coordination/v1beta1
k8s.io/api/discovery/v1
k8s.io/api/discovery/v1beta1
k8s.io/api/events/v1
k8s.io/api/events/v1beta1
k8s.io/api/extensions/v1beta1
k8s.io/api/flowcontrol/v1
k8s.io/api/flowcontrol/v1beta1
k8s.io/api/flowcontrol/v1beta2
k8s.io/api/flowcontrol/v1beta3
k8s.io/api/networking/v1
k8s.io/api/networking/v1alpha1
k8s.io/api/networking/v1beta1
k8s.io/api/node/v1
k8s.io/api/node/v1alpha1
k8s.io/api/node/v1beta1
k8s.io/api/policy/v1
k8s.io/api/policy/v1beta1
k8s.io/api/rbac/v1
k8s.io/api/rbac/v1alpha1
k8s.io/api/rbac/v1beta1
k8s.io/api/resource/v1alpha2
k8s.io/api/scheduling/v1
k8s.io/api/scheduling/v1alpha1
k8s.io/api/scheduling/v1beta1
k8s.io/api/storage/v1
k8s.io/api/storage/v1alpha1
k8s.io/api/storage/v1beta1
k8s.io/api/storagemigration/v1alpha1
k8s.io/client-go/kubernetes/scheme
k8s.io/apimachinery/pkg/apis/meta/v1beta1
k8s.io/apimachinery/pkg/apis/meta/internalversion
k8s.io/apimachinery/pkg/apis/meta/internalversion/scheme
k8s.io/apimachinery/pkg/runtime/serializer/streaming
k8s.io/client-go/pkg/apis/clientauthentication
k8s.io/apimachinery/pkg/version
k8s.io/client-go/pkg/version
golang.org/x/sys/unix
golang.org/x/term
k8s.io/client-go/pkg/apis/clientauthentication/v1
k8s.io/client-go/pkg/apis/clientauthentication/v1beta1
k8s.io/client-go/pkg/apis/clientauthentication/install
k8s.io/client-go/tools/clientcmd/api
k8s.io/client-go/tools/metrics
golang.org/x/oauth2/internal
golang.org/x/oauth2
k8s.io/utils/clock
k8s.io/apimachinery/pkg/util/wait
k8s.io/client-go/util/connrotation
container/heap
golang.org/x/time/rate
k8s.io/client-go/util/workqueue
k8s.io/client-go/transport
k8s.io/client-go/plugin/pkg/client/auth/exec
k8s.io/client-go/rest/watch
k8s.io/client-go/util/keyutil
k8s.io/client-go/util/cert
k8s.io/utils/clock/testing
k8s.io/client-go/util/flowcontrol
k8s.io/client-go/rest
k8s.io/client-go/metadata
k8s.io/api/apidiscovery/v2
k8s.io/api/apidiscovery/v2beta1
k8s.io/apimachinery/pkg/api/equality
k8s.io/apimachinery/pkg/apis/meta/v1/validation
k8s.io/apimachinery/pkg/api/validation
sigs.k8s.io/structured-merge-diff/v4/schema
k8s.io/kube-openapi/pkg/schemaconv
sigs.k8s.io/structured-merge-diff/v4/fieldpath
sigs.k8s.io/structured-merge-diff/v4/typed
sigs.k8s.io/structured-merge-diff/v4/merge
k8s.io/apimachinery/pkg/util/managedfields/internal
k8s.io/apimachinery/pkg/util/managedfields
database/sql/driver
github.com/google/uuid
github.com/munnerz/goautoneg
k8s.io/kube-openapi/pkg/cached
hash/adler32
compress/zlib
encoding/xml
github.com/emicklei/go-restful/v3/log
github.com/emicklei/go-restful/v3
k8s.io/kube-openapi/pkg/spec3
k8s.io/kube-openapi/pkg/common
k8s.io/kube-openapi/pkg/handler3
k8s.io/client-go/openapi
k8s.io/client-go/discovery
k8s.io/client-go/dynamic
k8s.io/client-go/restmapper
sigs.k8s.io/controller-runtime/pkg/client/apiutil
sigs.k8s.io/controller-runtime/pkg/log
sigs.k8s.io/controller-runtime/pkg/client
github.com/migtools/oadp-non-admin/pkg/velero/storagelocation/paths

@kaovilai kaovilai force-pushed the design/132 branch 2 times, most recently from fd3d671 to de07ca5 Compare February 7, 2025 04:13
Signed-off-by: Tiger Kaovilai <[email protected]>

paths implementation

Signed-off-by: Tiger Kaovilai <[email protected]>

CRD changes

Signed-off-by: Tiger Kaovilai <[email protected]>

design update

Signed-off-by: Tiger Kaovilai <[email protected]>

mpryc feedback

Signed-off-by: Tiger Kaovilai <[email protected]>
Signed-off-by: Tiger Kaovilai <[email protected]>
Signed-off-by: Tiger Kaovilai <[email protected]>
Signed-off-by: Tiger Kaovilai <[email protected]>
Signed-off-by: Tiger Kaovilai <[email protected]>
Signed-off-by: Tiger Kaovilai <[email protected]>
Signed-off-by: Tiger Kaovilai <[email protected]>
Signed-off-by: Tiger Kaovilai <[email protected]>
@kaovilai
Copy link
Member Author

kaovilai commented Feb 7, 2025

/hold for openshift/oadp-operator#1628

Signed-off-by: Tiger Kaovilai <[email protected]>
Signed-off-by: Tiger Kaovilai <[email protected]>
return true

// adds logsPath and resourceListPath to status if velero backup is completed.
if status.VeleroBackup.Status.Phase == velerov1.BackupPhaseCompleted {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not seem to comply with design. Here we only add info if backup succeeds. Should not check for finished state phases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the info isn't usable until backup completes. If it is unclear in design will fix.

If backup fails, (I guess partially fail could work), these urls won't work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence made me think that the goal is to add more info for debugging unsuccessful backups/restores
https://github.com/kaovilai/oadp-non-admin/blob/29286ec5cbc3021ba93f5013196d60e2fa98a611/docs/design/Debug_DownloadURL.md?plain=1#L13

maybe adding to Non Goals that backup/restore phases that will not contain the info

switch storageLocation.Spec.Provider {
case "aws":
protocol = "s3://"
// TODO: implement azure
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the plan is to implement this prior to first release?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to boss.

// TODO: implement gcp
//nolint:dupword // gcp format would be gs://my-bucket, see: https://cloud.google.com/sdk/gcloud/reference/storage/cp
default:
return "", errors.New("unimplemented provider")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can cause an endless loop

if NAB uses a gcp/azure NABSL/BSL, when its completes, reconcile will stuck in this part. Example of repeated log

  2025-02-13T08:40:39-03:00	ERROR	Reconciler error	{"controller": "nonadminrestore", "controllerGroup": "oadp.openshift.io", "controllerKind": "NonAdminRestore", "NonAdminRestore": {"name":"non-admin-restore-object-1","namespace":"test-non-admin-restore-reconcile-full-1"}, "namespace": "test-non-admin-restore-reconcile-full-1", "name": "non-admin-restore-object-1", "reconcileID": "26ae54b8-8729-4193-ad00-fd2e7aeac4c9", "error": "unimplemented provider"}

I would suggest changing implementation to when gcp/azure is detected, to not return an error. Could even populate that field in log/resourceList status field with message that this provider is not yet implemented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose download command/url to debug information perhaps for non-admin backup/restore ops
3 participants