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.

Sign up for GitHub

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

fix br log issue and include both 3.1 and 4.0 br in the tidb-backup-manager image #2213

Merged
merged 2 commits into from
Apr 16, 2020
Merged

fix br log issue and include both 3.1 and 4.0 br in the tidb-backup-manager image #2213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ff4cde8
fix br log issue and include both 3.1 and 4.0 br in the tidb-backup-m…
DanielZhangQD Apr 16, 2020
033cb39
fix ci
DanielZhangQD Apr 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions cmd/backup-manager/app/backup/backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@
package backup

import (
"bufio"
"context"
"fmt"
"io"
"io/ioutil"
"os/exec"
"path"
"strings"
Expand Down Expand Up @@ -64,29 +66,39 @@ func (bo *Options) backupData(backup *v1alpha1.Backup) (string, error) {
}
fullArgs = append(fullArgs, args...)
klog.Infof("Running br command with args: %v", fullArgs)
cmd := exec.Command("br", fullArgs...)
cmd.Stderr = cmd.Stdout
bin := "br" + backupUtil.Suffix(bo.TiKVVersion)
cmd := exec.Command(bin, fullArgs...)
Copy link
Contributor

Choose a reason for hiding this comment

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

if BR_LOG_TO_TERM should be always set, set it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is a way, I think the current setting is OK because we can see the env from the job clearly, if set it here, we cannot know it without going through the code or getting into the Pod.


stdOut, err := cmd.StdoutPipe()
if err != nil {
return remotePath, fmt.Errorf("cluster %s, create stdout pipe failed, err: %v", bo, err)
}
stdErr, err := cmd.StderrPipe()
if err != nil {
return remotePath, fmt.Errorf("cluster %s, create stderr pipe failed, err: %v", bo, err)
}
err = cmd.Start()
if err != nil {
return remotePath, fmt.Errorf("cluster %s, execute br command failed, args: %s, err: %v", bo, fullArgs, err)
}
var tmpOutput, errMsg string
var errMsg string
reader := bufio.NewReader(stdOut)
for {
tmp := make([]byte, 1024)
_, err := stdOut.Read(tmp)
tmpOutput = string(tmp)
if strings.Contains(tmpOutput, "[ERROR]") {
errMsg += tmpOutput
line, err := reader.ReadString('\n')
if strings.Contains(line, "[ERROR]") {
errMsg += line
}
klog.Infof(strings.Replace(tmpOutput, "\n", "", -1))
if err != nil {

klog.Infof(strings.Replace(line, "\n", "", -1))
if err != nil || io.EOF == err {
break
}
}
tmpErr, _ := ioutil.ReadAll(stdErr)
if len(tmpErr) > 0 {
klog.Infof(string(tmpErr))
errMsg += string(tmpErr)
}
err = cmd.Wait()
if err != nil {
return remotePath, fmt.Errorf("cluster %s, wait pipe message failed, errMsg %s, err: %v", bo, errMsg, err)
Expand Down
1 change: 1 addition & 0 deletions cmd/backup-manager/app/cmd/backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func NewBackupCommand() *cobra.Command {

cmd.Flags().StringVar(&bo.Namespace, "namespace", "", "Backup CR's namespace")
cmd.Flags().StringVar(&bo.ResourceName, "backupName", "", "Backup CRD object name")
cmd.Flags().StringVar(&bo.TiKVVersion, "tikvVersion", util.DefaultVersion, "TiKV version")
cmd.Flags().BoolVar(&bo.TLSClient, "client-tls", false, "Whether client tls is enabled")
cmd.Flags().BoolVar(&bo.TLSCluster, "cluster-tls", false, "Whether cluster tls is enabled")
return cmd
Expand Down
1 change: 1 addition & 0 deletions cmd/backup-manager/app/cmd/restore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func NewRestoreCommand() *cobra.Command {

cmd.Flags().StringVar(&ro.Namespace, "namespace", "", "Restore CR's namespace")
cmd.Flags().StringVar(&ro.ResourceName, "restoreName", "", "Restore CRD object name")
cmd.Flags().StringVar(&ro.TiKVVersion, "tikvVersion", util.DefaultVersion, "TiKV version")
cmd.Flags().BoolVar(&ro.TLSClient, "client-tls", false, "Whether client tls is enabled")
cmd.Flags().BoolVar(&ro.TLSCluster, "cluster-tls", false, "Whether cluster tls is enabled")
return cmd
Expand Down
33 changes: 23 additions & 10 deletions cmd/backup-manager/app/restore/restore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
package restore

import (
"bufio"
"fmt"
"io"
"io/ioutil"
"os/exec"
"path"
"strings"
Expand Down Expand Up @@ -58,29 +61,39 @@ func (ro *Options) restoreData(restore *v1alpha1.Restore) error {
}
fullArgs = append(fullArgs, args...)
klog.Infof("Running br command with args: %v", fullArgs)
cmd := exec.Command("br", fullArgs...)
cmd.Stderr = cmd.Stdout
bin := "br" + backupUtil.Suffix(ro.TiKVVersion)
cmd := exec.Command(bin, fullArgs...)

stdOut, err := cmd.StdoutPipe()
if err != nil {
return fmt.Errorf("cluster %s, create stdout pipe failed, err: %v", ro, err)
}
stdErr, err := cmd.StderrPipe()
if err != nil {
return fmt.Errorf("cluster %s, create stderr pipe failed, err: %v", ro, err)
}
err = cmd.Start()
if err != nil {
return fmt.Errorf("cluster %s, execute br command failed, args: %s, err: %v", ro, fullArgs, err)
}
var tmpOutput, errMsg string
var errMsg string
reader := bufio.NewReader(stdOut)
for {
tmp := make([]byte, 1024)
_, err := stdOut.Read(tmp)
tmpOutput = string(tmp)
if strings.Contains(tmpOutput, "[ERROR]") {
errMsg += tmpOutput
line, err := reader.ReadString('\n')
if strings.Contains(line, "[ERROR]") {
errMsg += line
}
klog.Infof(strings.Replace(tmpOutput, "\n", "", -1))
if err != nil {
klog.Infof(strings.Replace(line, "\n", "", -1))
if err != nil || io.EOF == err {
break
}
}
tmpErr, _ := ioutil.ReadAll(stdErr)
if len(tmpErr) > 0 {
klog.Infof(string(tmpErr))
errMsg += string(tmpErr)
}

err = cmd.Wait()
if err != nil {
return fmt.Errorf("cluster %s, wait pipe message failed, errMsg %s, err: %v", ro, errMsg, err)
Expand Down
1 change: 1 addition & 0 deletions cmd/backup-manager/app/util/generic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type GenericOptions struct {
Port int32
Password string
User string
TiKVVersion string
}

func (bo *GenericOptions) String() string {
Expand Down
27 changes: 26 additions & 1 deletion cmd/backup-manager/app/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,22 @@ import (
"os"
"strings"

"github.com/Masterminds/semver"
"github.com/spf13/pflag"
"k8s.io/klog"
cmdutil "k8s.io/kubectl/pkg/cmd/util"

"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
)

var (
cmdHelpMsg string
cmdHelpMsg string
supportedVersions = map[string]struct{}{
"3.1": {},
"4.0": {},
}
// DefaultVersion is the default tikv and br version
DefaultVersion = "4.0"
)

func validCmdFlagFunc(flag *pflag.Flag) {
Expand Down Expand Up @@ -162,3 +170,20 @@ func constructBRGlobalOptions(config *v1alpha1.BRConfig) []string {
}
return args
}

// Suffix parses the major and minor version from the string and return the suffix
func Suffix(version string) string {
numS := strings.Split(DefaultVersion, ".")
defaultSuffix := numS[0] + numS[1]

v, err := semver.NewVersion(version)
if err != nil {
klog.Errorf("Parse version %s failure, error: %v", version, err)
return defaultSuffix
}
parsed := fmt.Sprintf("%d.%d", v.Major(), v.Minor())
if _, ok := supportedVersions[parsed]; ok {
return fmt.Sprintf("%d%d", v.Major(), v.Minor())
}
return defaultSuffix
}
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ require (
github.com/Azure/go-autorest/autorest/mocks v0.3.0 // indirect
github.com/BurntSushi/toml v0.3.1
github.com/MakeNowJust/heredoc v0.0.0-20171113091838-e9091a26100e // indirect
github.com/Masterminds/semver v1.4.2
github.com/Microsoft/go-winio v0.4.12 // indirect
github.com/NYTimes/gziphandler v1.1.1 // indirect
github.com/ant31/crd-validation v0.0.0-20180702145049-30f8a35d0ac2
Expand Down
1 change: 1 addition & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ github.com/JeffAshton/win_pdh v0.0.0-20161109143554-76bb4ee9f0ab/go.mod h1:3VYc5
github.com/MakeNowJust/heredoc v0.0.0-20170808103936-bb23615498cd/go.mod h1:64YHyfSL2R96J44Nlwm39UHepQbyR5q10x7iYa1ks2E=
github.com/MakeNowJust/heredoc v0.0.0-20171113091838-e9091a26100e h1:eb0Pzkt15Bm7f2FFYv7sjY7NPFi3cPkS3tv1CcrFBWA=
github.com/MakeNowJust/heredoc v0.0.0-20171113091838-e9091a26100e/go.mod h1:64YHyfSL2R96J44Nlwm39UHepQbyR5q10x7iYa1ks2E=
github.com/Masterminds/semver v1.4.2 h1:WBLTQ37jOCzSLtXNdoo8bNM8876KhNqOKvrlGITgsTc=
github.com/Masterminds/semver v1.4.2/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y=
github.com/Microsoft/go-winio v0.4.11/go.mod h1:VhR8bwka0BXejwEJY73c50VrPtXAaKcyvVC4A4RozmA=
github.com/Microsoft/go-winio v0.4.12 h1:xAfWHN1IrQ0NJ9TBC0KBZoqLjzDTr1ML+4MywiUOryc=
Expand Down
32 changes: 22 additions & 10 deletions images/tidb-backup-manager/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,15 @@ FROM pingcap/tidb-enterprise-tools:latest
ARG VERSION=v1.51.0
ARG SHUSH_VERSION=v1.4.0
ARG TOOLKIT_VERSION=v3.0.12
ARG TOOLKIT_V31=v3.1.0-rc
ARG TOOLKIT_V40=v4.0.0-rc
Copy link
Contributor

Choose a reason for hiding this comment

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

s/TOOLKIT/BR/g

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But they are actually the toolkit version, BR binaries are included in the toolkit packages.
And also we may need to include other tools later in the toolkit packages.

Copy link
Contributor

Choose a reason for hiding this comment

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

TOOLKIT has alot of tools I think TOOLKIT is all right.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, we extract br from toolkit tarball but it's the implementation details. Your purpose is to specify the version of BR you wanted.

I can't understand the differences between these similar arguments.

ARG TOOLKIT_VERSION=v3.0.12
ARG TOOLKIT_V31=v3.1.0-rc
ARG TOOLKIT_V40=v4.0.0-rc

s/TOOLKIT/TOOLKIT_BR/g ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If with TOOLKIT_BR, what if we need to retrieve other tools, e.g. Lightning from this tarball later, as you said, br is implementation details, here the toolkit version is what I want.

RUN apk update && apk add ca-certificates

RUN wget -nv https://github.com/ncw/rclone/releases/download/${VERSION}/rclone-${VERSION}-linux-amd64.zip \
&& unzip rclone-${VERSION}-linux-amd64.zip \
&& mv rclone-${VERSION}-linux-amd64/rclone /usr/local/bin \
&& chmod 755 /usr/local/bin/rclone \
&& rm -rf rclone-${VERSION}-linux-amd64.zip rclone-${VERSION}-linux-amd64

RUN wget -nv http://download.pingcap.org/br-latest-linux-amd64.tar.gz \
&& tar -xzf br-latest-linux-amd64.tar.gz \
&& mv bin/br /usr/local/bin \
&& chmod 755 /usr/local/bin/br \
&& rm -rf br-latest-linux-amd64.tar.gz
&& unzip rclone-${VERSION}-linux-amd64.zip \
&& mv rclone-${VERSION}-linux-amd64/rclone /usr/local/bin \
&& chmod 755 /usr/local/bin/rclone \
&& rm -rf rclone-${VERSION}-linux-amd64.zip rclone-${VERSION}-linux-amd64

RUN wget -nv https://github.com/realestate-com-au/shush/releases/download/${SHUSH_VERSION}/shush_linux_amd64 \
&& mv shush_linux_amd64 /usr/local/bin/shush \
Expand All @@ -29,6 +25,22 @@ RUN \
&& rm -rf tidb-toolkit-${TOOLKIT_VERSION}-linux-amd64.tar.gz \
&& rm -rf tidb-toolkit-${TOOLKIT_VERSION}-linux-amd64

RUN \
wget -nv https://download.pingcap.org/tidb-toolkit-${TOOLKIT_V31}-linux-amd64.tar.gz \
&& tar -xzf tidb-toolkit-${TOOLKIT_V31}-linux-amd64.tar.gz \
&& mv tidb-toolkit-${TOOLKIT_V31}-linux-amd64/bin/br /usr/local/bin/br31 \
&& chmod 755 /usr/local/bin/br31 \
&& rm -rf tidb-toolkit-${TOOLKIT_V31}-linux-amd64.tar.gz \
&& rm -rf tidb-toolkit-${TOOLKIT_V31}-linux-amd64

RUN \
wget -nv https://download.pingcap.org/tidb-toolkit-${TOOLKIT_V40}-linux-amd64.tar.gz \
&& tar -xzf tidb-toolkit-${TOOLKIT_V40}-linux-amd64.tar.gz \
&& mv tidb-toolkit-${TOOLKIT_V40}-linux-amd64/bin/br /usr/local/bin/br40 \
&& chmod 755 /usr/local/bin/br40 \
&& rm -rf tidb-toolkit-${TOOLKIT_V40}-linux-amd64.tar.gz \
&& rm -rf tidb-toolkit-${TOOLKIT_V40}-linux-amd64

COPY bin/tidb-backup-manager /tidb-backup-manager
COPY entrypoint.sh /entrypoint.sh

Expand Down
15 changes: 15 additions & 0 deletions pkg/backup/backup/backup_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package backup

import (
"fmt"
"strings"

"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/backup"
Expand Down Expand Up @@ -269,6 +270,13 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s
return nil, fmt.Sprintf("failed to fetch tidbcluster %s/%s", backupNamespace, backup.Spec.BR.Cluster), err
}

var tikvVersion string
tikvImage := tc.TiKVImage()
imageVersion := strings.Split(tikvImage, ":")
if len(imageVersion) == 2 {
tikvVersion = imageVersion[1]
}

envVars, reason, err := backuputil.GenerateTidbPasswordEnv(ns, name, backup.Spec.From.SecretName, backup.Spec.UseKMS, bm.kubeCli)
if err != nil {
return nil, reason, err
Expand All @@ -280,12 +288,19 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s
}

envVars = append(envVars, storageEnv...)
envVars = append(envVars, corev1.EnvVar{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New version BR needs to set this env to print logs to stdout.

Name: "BR_LOG_TO_TERM",
Value: string(1),
})

args := []string{
"backup",
fmt.Sprintf("--namespace=%s", ns),
fmt.Sprintf("--backupName=%s", name),
}
if tikvVersion != "" {
args = append(args, fmt.Sprintf("--tikvVersion=%s", tikvVersion))
}

backupLabel := label.NewBackup().Instance(backup.GetInstanceName()).BackupJob().Backup(name)
volumeMounts := []corev1.VolumeMount{}
Expand Down
15 changes: 15 additions & 0 deletions pkg/backup/restore/restore_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package restore

import (
"fmt"
"strings"

"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
"github.com/pingcap/tidb-operator/pkg/backup"
Expand Down Expand Up @@ -254,6 +255,13 @@ func (rm *restoreManager) makeRestoreJob(restore *v1alpha1.Restore) (*batchv1.Jo
return nil, fmt.Sprintf("failed to fetch tidbcluster %s/%s", restoreNamespace, restore.Spec.BR.Cluster), err
}

var tikvVersion string
tikvImage := tc.TiKVImage()
imageVersion := strings.Split(tikvImage, ":")
if len(imageVersion) == 2 {
tikvVersion = imageVersion[1]
}

envVars, reason, err := backuputil.GenerateTidbPasswordEnv(ns, name, restore.Spec.To.SecretName, restore.Spec.UseKMS, rm.kubeCli)
if err != nil {
return nil, reason, err
Expand All @@ -265,11 +273,18 @@ func (rm *restoreManager) makeRestoreJob(restore *v1alpha1.Restore) (*batchv1.Jo
}

envVars = append(envVars, storageEnv...)
envVars = append(envVars, corev1.EnvVar{
Name: "BR_LOG_TO_TERM",
Value: string(1),
})
args := []string{
"restore",
fmt.Sprintf("--namespace=%s", ns),
fmt.Sprintf("--restoreName=%s", name),
}
if tikvVersion != "" {
args = append(args, fmt.Sprintf("--tikvVersion=%s", tikvVersion))
}

restoreLabel := label.NewBackup().Instance(restore.GetInstanceName()).RestoreJob().Restore(name)
volumeMounts := []corev1.VolumeMount{}
Expand Down